Repository navigation
fix(backup): restore sort keys, indexes, and throughput; bound memory; recover abandoned restores - #384
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
robinnsc
changed the title
Nothing stopped two `extenddb serve` processes from opening the same SQLite file. The backend serializes writers with a lock inside the server process, so two servers write concurrently and can hit SQLITE_BUSY mid-transaction; and startup recovery (control-plane transitions, GSI and vector index rebuilds, and, with #384, abandoned restores) assumes no other server is running, so a second server starting up would undo the first one's work in progress. Nothing stopped `destroy` from unlinking the file under a running server either, which left that server serving an unlinked inode while a later `init` created a fresh database at the same path. `extenddb serve` on a file database now takes an exclusive flock(2) on `<database>.lock` before opening the database and holds it for the life of the process (the lock lives in the engine, so it is held while any clone is). `init` and `migrate` take the same lock for their duration through the bootstrapper's migration-lock hook, and `destroy` takes it before it removes the file; each refuses, rather than waits, when a server holds it. Read-only commands (`settings`, `manage`, `verify`, `status`) take no lock. A second holder fails with: another extenddb process is already using <db> (lock held on <db>.lock); stop it first, or point this command at a different database The lock file is named after the file SQLite actually opens, not the configured string: the location is parsed with sqlx's own SqliteConnectOptions (so `sqlite:` URL forms, percent-encoding such as `a%20b.sqlite`, and `file:` URIs resolve as the engine resolves them), and the path is canonicalized, so two spellings of one file, a `..` path, or a symlink and its target all share one lock. A disk file whose name happens to contain `mode=memory` is a file. In-memory databases take no lock. The kernel releases the lock when the process exits, including on kill -9, so there is no stale-lock cleanup. On non-Unix platforms no lock is taken and a warning is logged. flock is advisory and its behaviour on network filesystems depends on the server and mount; the docs say to keep SQLite databases on local disk. The directory holding the database must be writable so the lock file can be created. flock through libc rather than std's File::try_lock, which needs Rust 1.89; the workspace MSRV is 1.88. libc is already a dependency of the workspace and listed in every license notices file. Docs: troubleshooting entry for the error with its scope; README and the deployment and design guides now state that one instance per catalog is the supported deployment, that SQLite enforces it, and that PostgreSQL and MongoDB do not (per-instance caches without cross-instance invalidation, every worker on every instance, a liveness-only /health). The deployment guide previously said multiple PostgreSQL instances were consistent because there was no in-process cache, which was not true. Tests: database_file resolution for plain paths, sqlite: URLs, percent-encoding, file: URIs, in-memory forms, and a disk file named like a memory parameter; ServeLock refused on the same file, independent across files, shared across `..` spellings and across file and directory symlinks; the server factory refusing a second server on one file; destroy and migrate refused while a server holds the file and a server refused while migrate holds it. Signed-off-by: Scott Robinson <robinnsc@amazon.com>
13c7f24 to
dc980eb
Compare
There was a problem hiding this comment.
Blocking on two things, both inline:
- the Postgres CreateBackup barrier doesn't pin the snapshot the way the code and docs say it does, and
- SQLite CreateBackup now holds the write lock for the whole read, which is a server-wide write stall on a big table. The rest are non-blocking.
| .execute(&mut *snapshot) | ||
| .await | ||
| .map_err(db_err)?; | ||
| meta.commit().await.map_err(db_err)?; |
There was a problem hiding this comment.
Blocking. This barrier doesn't hold. LOCK TABLE is a utility statement and doesn't take a snapshot in Postgres. The REPEATABLE READ snapshot is taken by the first SELECT, which here runs after meta.commit() has already released the FOR SHARE row. So the definition is captured at one instant and the items at a later one, with UpdateTable free to commit in between. I checked this on PG 15: BEGIN ... REPEATABLE READ; LOCK TABLE t ...; then an insert from another session, then SELECT count(*) sees the new row.
There was a problem hiding this comment.
Yeah good catch, swapped that to BEGIN RR RO; LOCK TABLE … ACCESS SHARE; SELECT 1 FROM <data> LIMIT 1; and only then releases the FOR SHARE row
| // instant, including against a concurrent UpdateTable. Items are | ||
| // read in rowid batches on the same connection, so memory is | ||
| // bounded by the batch size. | ||
| let _writer = self.write_lock.lock().await; |
There was a problem hiding this comment.
Blocking. This now holds the engine write lock for the whole read and write of the table. On main the item read ran before the lock, only the inserts were under it. The pool is WAL mode, so a deferred read transaction on a second connection already gives a stable snapshot without blocking writers.
There was a problem hiding this comment.
FIxed, the lock is now held only to read the definition, insert the backup row as CREATING, and open the deferred read transaction (snapshot fixed under the lock, so definition and items agree), items stream unlocked and are written in batches under short holds
| "SSESpecificationOverride", | ||
| ]; | ||
|
|
||
| fn validate_restore_table_overrides(body: &Value) -> Result<(), DynamoDbError> { |
There was a problem hiding this comment.
Now that the definition is in hand, BillingModeOverride and ProvisionedThroughputOverride are a few lines on CreateTableInput. Refusing them is a wire regression for clients that pass them (the console does). I'd implement at least those two and refuse only the index and SSE overrides.
| | `RestoreTableFromBackup` overrides | `BillingModeOverride`, `GlobalSecondaryIndexOverride`, `LocalSecondaryIndexOverride`, `ProvisionedThroughputOverride`, `SSESpecificationOverride` apply to the restored table | Not supported; a request carrying any of them returns `ValidationException` rather than restoring a table that differs from what was asked for | | ||
| | Multi-part base table keys (preview) | Not supported | A backup of a table created with `enable_multipart_keys` is refused at restore with `ValidationException`; the item paths address such tables by their first HASH and RANGE attribute only | | ||
| | `DeleteBackup` during a restore from that backup | `BackupInUseException` | Matches: `BackupInUseException` (HTTP 400) until the restore completes or fails | | ||
| | `DeleteTable` on a table being restored | `ResourceInUseException` | Matches on a restore target. An ordinary table in `CREATING` can still be deleted during its control-plane delay, where the service refuses | |
There was a problem hiding this comment.
This says "Matches on a restore target" without a backend qualifier, but the MongoDB diff only adds RestoreSummary and the DeleteBackup refusal. DeleteTable during a Mongo restore is not refused, and Mongo has no abandoned-restore sweep. Either qualify these rows or add the behaviour.
There was a problem hiding this comment.
Qualified every row by backend, and implemented the Mongo DeleteTable, and clarified that Mongo still has no abandoned restore sweep
| ## Version History | ||
|
|
||
| ### Catalog 0.0.3 (Current) | ||
| ### Catalog 0.0.4 (Current) |
There was a problem hiding this comment.
Worth a sentence here: any Postgres target the old bug left in CREATING matches the sweep predicate and gets removed on the first control-plane pass after upgrade. Right outcome, but operators will see tables disappear from ListTables and should know why.
| } | ||
| let rows_sql = format!( | ||
| "SELECT rowid, item_data FROM {table} \ | ||
| WHERE {cond} AND rowid > ? AND rowid <= ? ORDER BY rowid" |
There was a problem hiding this comment.
The restore cursor is the implicit rowid of backup_items, read across separately committed batches. backup_items has no INTEGER PRIMARY KEY, so a VACUUM between batches can renumber rowids and the restore skips or duplicates rows. Nothing in-tree runs VACUUM and #391 keeps a second extenddb out, but not a sqlite3 CLI. Give backup_items an INTEGER PRIMARY KEY or cursor on something stable.
There was a problem hiding this comment.
Done, backup_items has id INTEGER PRIMARY KEY and the restore cursors on it. An existing table is rebuilt in place by migrate, and the fixture test now checks id is present with rows in order.
| // This session sits idle for the whole copy. A server-side | ||
| // idle_session_timeout would end it and release the lock while the | ||
| // restore is still running, so it is disabled for this session only. | ||
| sqlx::query("SET idle_session_timeout = 0") |
There was a problem hiding this comment.
If this SET fails (managed instance that rejects it), every restore becomes a 500. Make it best-effort with a warning.
There was a problem hiding this comment.
Done, now best-effort with a warning
| // mid-copy leaves its target CREATING with no scheduled transition, | ||
| // which nothing above would ever move on. Logged and skipped on | ||
| // failure, so it cannot hold up the transitions above. | ||
| match self.sweep_abandoned_restores().await { |
There was a problem hiding this comment.
This pass is notify-driven with a 60 s idle timeout, so an abandoned target is removed between 60 and ~120 s, not "after 60 s". Each pass also opens a fresh out-of-pool connection per still-locked candidate. Fine, but say so in the comment.
There was a problem hiding this comment.
Corrected the comment
| .map_err(|e| StorageError::Internal(format!("Database error: {e}")))?; | ||
| .map_err(db_err)?; | ||
| sqlx::query(&format!( | ||
| "LOCK TABLE {} IN ACCESS SHARE MODE", |
There was a problem hiding this comment.
This lock is now held across the read and the interleaved catalog writes. A DeleteTable issued during the backup reaches the control-plane worker, whose DROP TABLE blocks behind it and stalls the whole worker pass until the backup finishes. Main had the same shape for the read only. Either SET LOCAL lock_timeout in the worker's drop or document it.
There was a problem hiding this comment.
Went with using SET LOCAL lock_timeout
| .unwrap_or(i64::MAX); | ||
| sqlx::query( | ||
| "INSERT INTO backup_items (backup_arn, pk, sk, item_data) \ | ||
| VALUES (?, '', NULL, ?)", |
There was a problem hiding this comment.
SQLite stores pk = '' while Postgres stores the real pk. Harmless today but the #348 migration-from-rows will want it. Also COPY_BATCH_ITEMS is imported but this file uses its own COPY_BATCH.
There was a problem hiding this comment.
Swapped to storing the Real pk
Nothing stopped two `extenddb serve` processes from opening the same SQLite file. The backend serializes writers with a lock inside the server process, so two servers write concurrently and can hit SQLITE_BUSY mid-transaction; and startup recovery (control-plane transitions, GSI and vector index rebuilds, and, with #384, abandoned restores) assumes no other server is running, so a second server starting up would undo the first one's work in progress. Nothing stopped `destroy` from unlinking the file under a running server either, which left that server serving an unlinked inode while a later `init` created a fresh database at the same path. `extenddb serve` on a file database now takes an exclusive flock(2) on `<database>.lock` before opening the database and holds it for the life of the process (the lock lives in the engine, so it is held while any clone is). `init` and `migrate` take the same lock for their duration through the bootstrapper's migration-lock hook, and `destroy` takes it before it removes the file; each refuses, rather than waits, when a server holds it. `init` holds it through the encryption key, default account, and admin user, not only the schema, so a server cannot start in the window where the schema exists but the key does not. Read-only commands (`settings`, `manage`, `verify`, `status`) take no lock. A second holder fails with: another extenddb process is already using <db> (lock held on <db>.lock); stop it first, or point this command at a different database The lock file is named after the file SQLite actually opens, not the configured string: the location is parsed with sqlx's own SqliteConnectOptions (so `sqlite:` URL forms, percent-encoding such as `a%20b.sqlite`, and `file:` URIs resolve as the engine resolves them), and the path is canonicalized, so two spellings of one file, a `..` path, or a symlink and its target all share one lock. A dangling symlink (the target not created yet) is followed by hand, since canonicalize refuses it. A disk file whose name happens to contain `mode=memory` is a file. In-memory databases take no lock. The lock file is created 0600 and an existing one is tightened to 0600 on open, matching the database and its sidecars: flock needs only a read handle, so a world-readable lock file would let any local user hold the lock and keep the server from starting. The kernel releases the lock when the process exits, including on kill -9, so there is no stale-lock cleanup. On non-Unix platforms no lock is taken and a warning is logged. flock is advisory and its behaviour on network filesystems depends on the server and mount; the docs say to keep SQLite databases on local disk. The directory holding the database must be writable so the lock file can be created. flock through libc rather than std's File::try_lock, which needs Rust 1.89; the workspace MSRV is 1.88. libc is already a dependency of the workspace and listed in every license notices file. Docs: troubleshooting entry for the error with its scope; README and the deployment and design guides now state that one instance per catalog is the supported deployment, that SQLite enforces it, and that PostgreSQL and MongoDB do not (per-instance caches without cross-instance invalidation, every worker on every instance, a liveness-only /health). The deployment guide previously said multiple PostgreSQL instances were consistent because there was no in-process cache, which was not true. Tests: database_file resolution for plain paths, sqlite: URLs, percent-encoding, file: URIs, in-memory forms, and a disk file named like a memory parameter; ServeLock refused on the same file, independent across files, shared across `..` spellings, across file and directory symlinks, and across a dangling symlink and its future target (one and two links); the lock file created 0600 and an existing 0644 one tightened; the server factory refusing a second server on one file; destroy and migrate refused while a server holds the file and a server refused while migrate holds it. Signed-off-by: Scott Robinson <robinnsc@amazon.com>
Nothing stopped two `extenddb serve` processes from opening the same SQLite file. The backend serializes writers with a lock inside the server process, so two servers write concurrently and can hit SQLITE_BUSY mid-transaction; and startup recovery (control-plane transitions, GSI and vector index rebuilds, and, with ExtendDB#384, abandoned restores) assumes no other server is running, so a second server starting up would undo the first one's work in progress. Nothing stopped `destroy` from unlinking the file under a running server either, which left that server serving an unlinked inode while a later `init` created a fresh database at the same path. `extenddb serve` on a file database now takes an exclusive flock(2) on `<database>.lock` before opening the database and holds it for the life of the process (the lock lives in the engine, so it is held while any clone is). `init` and `migrate` take the same lock for their duration through the bootstrapper's migration-lock hook, and `destroy` takes it before it removes the file; each refuses, rather than waits, when a server holds it. `init` holds it through the encryption key, default account, and admin user, not only the schema, so a server cannot start in the window where the schema exists but the key does not. Read-only commands (`settings`, `manage`, `verify`, `status`) take no lock. A second holder fails with: another extenddb process is already using <db> (lock held on <db>.lock); stop it first, or point this command at a different database The lock file is named after the file SQLite actually opens, not the configured string: the location is parsed with sqlx's own SqliteConnectOptions (so `sqlite:` URL forms, percent-encoding such as `a%20b.sqlite`, and `file:` URIs resolve as the engine resolves them), and the path is canonicalized, so two spellings of one file, a `..` path, or a symlink and its target all share one lock. A dangling symlink (the target not created yet) is followed by hand, since canonicalize refuses it. A disk file whose name happens to contain `mode=memory` is a file. In-memory databases take no lock. The lock file is created 0600 and an existing one is tightened to 0600 on open, matching the database and its sidecars: flock needs only a read handle, so a world-readable lock file would let any local user hold the lock and keep the server from starting. The kernel releases the lock when the process exits, including on kill -9, so there is no stale-lock cleanup. On non-Unix platforms no lock is taken and a warning is logged. flock is advisory and its behaviour on network filesystems depends on the server and mount; the docs say to keep SQLite databases on local disk. The directory holding the database must be writable so the lock file can be created. flock through libc rather than std's File::try_lock, which needs Rust 1.89; the workspace MSRV is 1.88. libc is already a dependency of the workspace and listed in every license notices file. Docs: troubleshooting entry for the error with its scope; README and the deployment and design guides now state that one instance per catalog is the supported deployment, that SQLite enforces it, and that PostgreSQL and MongoDB do not (per-instance caches without cross-instance invalidation, every worker on every instance, a liveness-only /health). The deployment guide previously said multiple PostgreSQL instances were consistent because there was no in-process cache, which was not true. Tests: database_file resolution for plain paths, sqlite: URLs, percent-encoding, file: URIs, in-memory forms, and a disk file named like a memory parameter; ServeLock refused on the same file, independent across files, shared across `..` spellings, across file and directory symlinks, and across a dangling symlink and its future target (one and two links); the lock file created 0600 and an existing 0644 one tightened; the server factory refusing a second server on one file; destroy and migrate refused while a server holds the file and a server refused while migrate holds it. Signed-off-by: Scott Robinson <robinnsc@amazon.com>
dc980eb to
8f83169
Compare
8f83169 to
ba9f869
Compare
|
#395 (SnapshotEngine) is branched from this PR and depends on it; its diff shows this PR's commit until this merges. |
…t code Revision of docs/design/15-backup-restore.md after reviewing the draft against main 9f43d03 and PRs #384, #335, #336, #337, measuring the read, compression, and bulk-load paths on PostgreSQL 16 and SQLite with a 1M-item fixture, and two review rounds against the companion PRs #393, #394 and #395. The revision log at the end of the document lists each change; the ones that alter behavior: - SnapshotEngine returns the table definition together with the items from one fenced operation (TableSnapshot). The items-only signature in the draft allowed a GSI change or DeleteTable between the definition read and the data snapshot. Indexes are reported in whatever build state they had and only an index being deleted is dropped; an ACTIVE-only filter would have left a GSI mid-backfill out of the backup for good. - The snapshot is opened in the CreateBackup request so the backup represents the request time, as DynamoDB documents; backup_creation_date_time is the instant the snapshot pinned. - Per-job claims with fencing replace the single-instance lease the draft assumed; PostgreSQL and MongoDB have no such lease. The claim is an advisory lock on a dedicated catalog connection on PostgreSQL (the seed #384's restore lock already uses), the row itself on MongoDB, an in-process slot on SQLite; no row ever exists unowned. Heartbeats are a timer, staleness is measured on the database's clock, and a failed heartbeat aborts the job. The reconciler resolves a dead CREATING row at once under a fresh generation conditioned on status = CREATING and deletes its prefix only after the stale window. - DeleteBackup tombstones the row as DELETING, deletes the manifest first, and marks DELETED only when the prefix is empty; backup sync is scoped to the configured store and never adopts a CREATING, DELETING, DELETED or FAILED prefix. Failed creations and deleted backups are BackupNotFound, as on the service, and share one retention. - Restore returns CREATING from the request (table and restore_jobs row created atomically; ResourceInUseException on a name race, LimitExceededException over the job limit), spools and verifies every data file before writing any item, and records progress in a way a crash cannot split from the data: a data-database row per file on PostgreSQL (data migration 005 reserved), the same transaction per 10k-item batch on SQLite, idempotent re-application of one file on MongoDB followed by a document-count check against the manifest. Duplicate detection for unordered backups is the target's primary key, not an in-memory key set. The ACTIVE flip is conditional; DeleteTable on a restore target fails the job. - The manifest self-checksum is replaced by a sidecar over the stored bytes; the draft's canonical form depended on serde_json formatting. backup_arn is the backup's identity and source_table must agree with it. N key values are normalized before key text or key order is computed; the SQL ORDER BY recipe includes COLLATE "C" on PostgreSQL. - Index population on restore stays per backend via bulk_load; the shared restore-backfill worker the draft described exists only on MongoDB. - Format additions: item_order, per-file key_range, md5 and uncompressed_bytes, explicit nulls, three-state vector_indexes of creation specs, optional etag, dataFileS3Key as the full object key, partition-aware BackupArn as the single ARN implementation. - Filesystem store: owner-only root re-checked by every operation, owner-only rule on every descended directory including list and delete_prefix, ancestor ownership rule, objects 0600 and directories 0700 at creation, put returns object metadata; the consequence for backup pickup agents is stated. The S3 conformance suite is mandatory in CI against moto (MinIO no longer publishes images). - ListBackups paging and filtering, the restore override contract, sqlite-memory behavior, extenddb destroy leaving the store intact, a restore spool outside the store root, max_concurrent_jobs, heartbeat_interval_secs and a concrete pool-size rule are specified. - Phase 1 is rewritten as the PR series with acceptance criteria and GitHub numbers. Migration numbers are reserved up front so the PITR PR is not on the critical path. Co-authored-by: Lee Hannigan <lhnng@amazon.com>
LeeroyHannigan
left a comment
LeeroyHannigan
left a comment
There was a problem hiding this comment.
Requesting changes. The storage engineering under this PR held under reproduction: both of yesyayen's blocking items are fixed and each fix fails its test when reverted; restore atomicity against DeleteBackup held on PostgreSQL in both orders and across 100 race iterations on MongoDB; the migration, the sweep predicate, the backup_items cursor, the downgrade gate, and crash cleanup all behaved as described; cross-account access to backups is refused on every backend under every ARN variation tried; the restored data matched DynamoDB item for item on every index and projection. The problems are in the surface customers and operators touch, and four of them block the merge.
Blocking
1. The merged tree does not compile. #338 landed the Cassandra backend on main this morning with the pre-PR four-argument restore_table_from_backup. cargo check --workspace --all-targets on the merge of origin/main and ba9f869 fails in one crate, extenddb-storage-cassandra, E0050 (4 parameters, trait has 5). GitHub reports MERGEABLE because no file overlaps. Adding the parameter alone leaves three defects: the Cassandra restore hardcodes global_secondary_indexes: None and local_secondary_indexes: None, ignores both overrides, and never calls ensure_single_part_base_key or ensure_restorable, because those refusals live in each SQL storage backend rather than in the engine. Either port the definition, the overrides, and both refusals into Cassandra, or add a Cassandra row to docs/differences-from-dynamodb.md saying what its restore does not honour. In both cases move the two refusals into crates/engine/src/backup.rs so a backend cannot skip them.
2. RestoreTableFromBackup copies every item inside the request on PostgreSQL and SQLite, and a client timeout cancels it. Both backends await copy_backup_items and only then return CREATING for a table that is already ACTIVE (the comment at backup_engine.rs:1282 says so). Reproduced with 40,000 items and two GSIs, client read timeout 3 s, no retries. PostgreSQL: the target sat CREATING with zero items, then the sweep removed it 64 s after the disconnect; the restore was lost. SQLite: the target is still CREATING four minutes later, a retry into the same name returns ResourceInUseException, DeleteTable on it returns ResourceInUseException ("Table is being restored"), and DeleteBackup on the source returns BackupInUseException. The customer is locked out of the name and the backup until the process restarts. boto3 and the CLI default to a 60 s read timeout, and with one INSERT per item per index over a network a few hundred thousand items cross it, so large restores cannot succeed with stock clients. SQLite CreateBackup in this PR already detaches its copy with tokio::spawn for this reason (backup.rs:799). Do the same here on both backends: commit the CREATING target and its table_restores row, spawn the copy, return at once. Add a live test that drops the client mid-restore and asserts ACTIVE with the full count. This also makes the DeleteBackup and DeleteTable refusals observable the way the docs describe them.
3. DescribeBackup still returns the pre-PR subset. Against DynamoDB, a backup of a table with a GSI, an LSI, and TTL enabled returns SourceTableFeatureDetails with GlobalSecondaryIndexes, LocalSecondaryIndexes, and TimeToLiveDescription, and SourceTableDetails.ProvisionedThroughput ({0, 0} for on-demand; the member is required in the shape). ExtendDB omits all of them on all three backends, although backup_definitions now holds the data. A customer auditing a backup is told it has no indexes; a strongly typed SDK can fail to deserialize the response. Populate both from the stored definition.
4. The PR body and the docs state behaviour the build does not have. Checked on servers built from ba9f869: "DeleteTable on a table that is being restored returns ResourceInUseException on all three backends" (only MongoDB; see item 2); "SQLite similarly exposes CREATING while it copies" (the caller blocks until AVAILABLE; only a concurrent observer sees CREATING); "a backup is never stuck undeletable" (false after a MongoDB crash during $out, and after item 2 on SQLite); "backups of tables with vector indexes are refused" (only the restore is refused; CreateBackup returns an AVAILABLE backup). Every claim in the body and in differences-from-dynamodb.md needs a live check per backend before the next revision.
Wire behaviour, measured against DynamoDB
Each of these was run on the service and on all three ExtendDB backends.
RestoreSummary.RestoreDateTimeis the restore wall clock. The service reports the source backup's creation time (measured:RestoreDateTime1791976431.438 equal toBackupCreationDateTime, restore issued 74 s later). A client reading it to learn which point in time was restored gets the wrong instant.- The service drops
RestoreSummaryfrom DescribeTable once the table is ACTIVE (absent at ACTIVE and 30 s later). ExtendDB keeps it withRestoreInProgress: false, andtests/test_backup_restore_fidelity.py:252-254pins that. RestoreSummary.SourceTableArnis never emitted; the service sets it.- The CREATING restore response carries GSI
IndexStatus: ACTIVE; the service omits the member. - A malformed
BackupArnreturns AccessDeniedException; the service returns ValidationException. - Restoring into an existing table name returns ResourceInUseException; the service returns TableAlreadyExistsException.
- An empty
GlobalSecondaryIndexOverrideorLocalSecondaryIndexOverridelist and anSSESpecificationOverrideare refused; the service accepts them as no-ops. The blanket refusal is documented, the no-op case is not. - The PROVISIONED-with-GSIs refusal tells the caller to pass
GlobalSecondaryIndexOverride, which ExtendDB then refuses. The fidelity test pins that text at line 649. ListBackupsignoresLimitand never returnsLastEvaluatedBackupArn; every row comes back in one response. The service caps at 100 with a token.
PostgreSQL
- The restore's ownership is a session advisory lock on a dedicated connection that issues no statement for the whole copy.
SET idle_session_timeout = 0is best-effort and the comment atbackup_engine.rs:118-123says managed services may reject it. On RDS, Aurora, or behind a transaction-mode pooler the idle session can end while the copy continues; after 60 s the sweep flips the target to DELETING and the restore fails with "deleted while it was being restored". Heartbeat the lock session, or gate the sweep on a liveness signal that does not depend on an idle connection. Item 2 does not remove this; a long detached copy has the same exposure. - "Bounded memory" bounds the Rust buffer. The copy is one PostgreSQL transaction (
beginat line 325, onecommitafter the loop), and secondary-index rows are written one INSERT per item per index in the nested loop at line 464 while base rows are batched. A million-item table with four GSIs is one transaction with four million round trips, WAL held throughout, target pinned CREATING throughout. - A failure to open the lock connection (
max_connectionsreached) maps to an Internal error. Map it to LimitExceeded so clients back off. restore_table_to_point_in_time(lines 1393-1420) is unreachable, uses a fixed backup name, and discards its cleanup error. Delete it.
SQLite and MongoDB
- CreateBackup on a table with a vector index returns an AVAILABLE backup that RestoreTableFromBackup always refuses. The customer finds out at restore time. Refuse at CreateBackup with the same message.
- MongoDB: a crash during the base
$outcopy, beforerestore_backfill_pendingis set, leaves a CREATING target that nothing sweeps or resumes. DeleteTable refuses it and DeleteBackup of the source returns BackupInUse until the catalog is edited by hand. Crashes after$outself-heal. Add a sweep for CREATING targets carryingrestore_source_backup_arnwith no pending flag and no live operation, or let DeleteTable remove them. - MongoDB ignores
--data-dband--catalog-dband has no serve lock, so two processes on one replica set share one catalog, andsweep_stale_backup_claimsat startup clears the other process's in-flight markers. Pre-existing, but the sweep makes it matter. pool_size = 1is raised to 2 for a file-backed database with no log line and no mention in the config docs. A file-backed backup also pins one pooled connection for its duration, sopool_size >= 3is wanted for undisturbed concurrency; say so.BackupMetadataClaim's Drop release only runs when a runtime handle is current; a comment should say the startup sweep is the backstop.- The MongoDB backup is not a point-in-time snapshot (
$outreads the live collection). Line 93 says so in the general consistency row; the Backup and Restore section should state the per-item mixed-state consequence where an operator choosing MongoDB will read it.
Tenancy and validation
BackupTableDefinition::apply_toapplies the stored definition withoutvalidate_create_table. A definition with 30 GSIs, a GSI on an undefined attribute, 101 projected attributes, or negative throughput restores to an ACTIVE table that CreateTable would reject. Triggering it needs catalog write access; it is still wrong data stored, and the service re-validates.RESTORE_SLOTSis a process-global semaphore of four. One account can hold all four and every other account's restores wait 30 s and get LimitExceededException. Make the limit per account.converge_catalog_versionandrefuse_newer_catalogtreat a storedcatalog_versionthe binary cannot parse as "proceed" and stamp the compiled version over it (older_or_equal_or_absent_is_allowedassertsSome("garbage")is allowed). That defeats the downgrade guard in the one case it exists for. Fail closed.- Pre-existing, outside this diff:
RestoreTableFromBackup,DescribeBackup, andDeleteBackupare authorized againstarn:...:table/*becauseextract_table_namereads onlyTableName(crates/server/src/request_helpers.rs:42-46, 289-294; the server log showsresource="...:table/*"on a denial). A policy scoped to a target table or a backup ARN can never match. The service authorizes on the backup ARN and the target table. This PR makes restore a first-class feature on top of that gap; fix it here or file an issue; either way it needs recording.
Upgrade path and docs
Walked literally from docs/manuals/07-upgrade-manual.md on a 0.0.3 deployment created by the 0.1.13 binary.
extenddb migrate --config extenddb.tomlas written fails: it needs PostgreSQL admin credentials the manual never mentions, and the error isError: Internal("Internal(\"Cannot connect as admin: error returned from database: password authentication failed for user \"lhnng\"\")"). Document--pg-userandEXTENDDB_PG_PASSWORDin the command, flatten the nested error, and say what to pass.- The manual says to run
migratewithout--yesfirst to see what is pending; the tool prints the plan and thenError: --yes is required to apply migrationswith exit status 1, so a scripted upgrade underset -estops at the preview. Exit 0 for the preview, or a documented distinct code. - On SQLite the upgrade rebuilds
backup_itemsby copying every row into a new table in one write transaction (schema.rs:494-553). No manual mentions it, the dry run does not name it, and the function logs nothing. Document it, name it in the dry run when the old shape is present, and log start and finish with the row count. - The sweep that removes targets the old bug left in CREATING logs nothing. Operators will see tables vanish from ListTables with no server record of why. Log each removal by name.
- The manual's backup and rollback commands hardcode
extenddb_catalog,extenddb, andOWNER extenddb;initlets the operator choose all three. There is no SQLite subsection (stop, copy the file, migrate, roll back by restoring the copy). - 0.0.4 and migration 003 are also claimed by #368 and #335. The migration ledger keys on filename and the gate compares version numbers, so if #368 merges first and stamps 0.0.4, this binary applies its own 003 and converges with no refusal: two schemas named 0.0.4. Whoever merges second renumbers to 004 and 0.0.5 (and the two assertions in
migrations.rstests, and the manual's 0.0.4 section). The order should be agreed now. differences-from-dynamodb.mdnames three backends; the merged tree ships four.- The
Storagetrait and the on-disk format changed.CONTRIBUTING.mdanddocs/rfcs/README.mdrequire an accepted RFC for both; the PR links #348, a design PR, not a numbered accepted RFC. - The PR template asks for a
## Breaking changessection; the changes are spread through the prose, and the one customers will hit first (overrides that were silently ignored are now hard errors) deserves its own line.
Minor
Backup ARN timestamp is 13 digits where the service zero-pads to 14; BackupCreationDateTime is whole seconds where the service is sub-second; the restore response adds LastUpdateToPayPerRequestDateTime, which the service omits; ListBackups summaries omit TableId; error __type namespace differs for ValidationException and AccessDeniedException; with two invalid inputs the service checks the ARN account before the target name.
| account_id: &str, | ||
| target_table_name: &str, | ||
| backup_arn: &str, | ||
| overrides: RestoreTableOverrides, |
There was a problem hiding this comment.
main now has crates/storage-cassandra/src/backup_engine.rs (#338, merged this morning) with the old four-argument signature, so the merge fails to compile: cargo check --workspace --all-targets on origin/main plus this branch gives E0050 in extenddb-storage-cassandra and nothing else. Adding the parameter there leaves three defects: the Cassandra restore hardcodes global_secondary_indexes: None and local_secondary_indexes: None, ignores the overrides, and never calls ensure_single_part_base_key or ensure_restorable, which this PR enforces inside the PostgreSQL and SQLite backends rather than in the engine. Port the definition and both refusals into Cassandra, or document in the differences matrix what its restore does not honour, and move the two refusals into the engine so a backend cannot skip them.
There was a problem hiding this comment.
Fixed. The trait now takes a resolved RestoreRequest. The engine runs ensure_restorable, restore_input and validate_create_table for every backend, and Cassandra restores GSIs, LSIs and the overrides through its V005 migration. cargo check --workspace --all-targets is clean on the merge.
| // CREATING with no scheduled transition, so nothing else would | ||
| // move it on, and the name would stay taken. Deleting the target | ||
| // cascades its table_restores row. | ||
| let copied = self.copy_backup_items(&desc, &backup_arn).await; |
There was a problem hiding this comment.
The copy is awaited inside the request future, so the response at line 1282 says CREATING for a table that is already ACTIVE, and a client that times out drops the future mid-copy. Reproduced with 40,000 items and two GSIs, read timeout 3 s, no retries: the target sat CREATING with zero items and the sweep removed it 64 s after the disconnect. The restore was lost. boto3 and the CLI default to 60 s, so any restore whose copy takes longer cannot succeed with a stock client. Commit the CREATING target and its table_restores row, spawn the copy, and return at once, the way SQLite CreateBackup does at backup.rs:799. Add a live test that drops the client and asserts the target reaches ACTIVE with the full count.
There was a problem hiding this comment.
Fixed. Registration and copy run in a spawned task; the request waits only for the CREATING description. Once the request is authenticated and authorized, the whole handler (backup read, validation, registration) also runs in its own task, so a disconnect does not cancel it part-way. Live tests: a 1 ms-read-timeout disconnect and the copy window (DeleteTable/DeleteBackup refused while CREATING) on all four backends, plus storage-level tests that drop the request future.
| // last one; until then the target is CREATING and refuses every | ||
| // data-plane request. A failure part-way is cleaned up here, a | ||
| // crash part-way by the startup sweep. | ||
| let copied = self.copy_backup_items(&desc, &backup_arn).await; |
There was a problem hiding this comment.
The SQLite path has the same shape. With the client gone after 2 s on a 40,000-item restore, the target is still CREATING with RestoreInProgress: true four minutes later; a retry into the same name returns ResourceInUseException, DeleteTable on the target returns ResourceInUseException ("Table is being restored"), and DeleteBackup on the source returns BackupInUseException. SQLite sweeps abandoned restores only at startup, so the name and the backup are locked until the process restarts. create_backup at line 799 already detaches its copy with tokio::spawn and the comment there names this hazard; the restore path needs the same treatment.
There was a problem hiding this comment.
Fixed the same way as PostgreSQL. serve also now refuses a file database left in a pre-release 0.0.4 shape and points to extenddb migrate --yes.
| // restore is still running, so disable it for this session when the | ||
| // server permits. Some managed PostgreSQL services reject this SET; | ||
| // ownership still works there, but remains subject to their idle timeout. | ||
| if let Err(e) = sqlx::query("SET idle_session_timeout = 0") |
There was a problem hiding this comment.
This connection issues no statement after pg_try_advisory_lock for the whole copy. When the SET is refused (the comment says managed services refuse it) or the connection sits behind a transaction-mode pooler, the idle session can end while the copy continues, the lock frees, and after 60 s the sweep flips the target to DELETING; the restore then fails with "deleted while it was being restored". That covers RDS, Aurora, and PgBouncer. Heartbeat the session while the copy runs, or gate the sweep on a liveness signal that does not depend on an idle connection.
There was a problem hiding this comment.
The lock session now runs SELECT 1 every 15 s and the copy stops if that fails, which covers idle-session reaping on RDS/Aurora. A transaction-mode pooler is not supported for restores (the lock is a session advisory lock): the admin guide says so, and the comments that implied poolers were handled are removed. Replacing the lock with a lease row (as MongoDB and Cassandra use) is the durable fix if transaction pooling matters; I left that out of this PR.
| "Backup not found: {backup_arn}" | ||
| ))); | ||
| } | ||
| let mut tx = self.data_pool.begin().await.map_err(db_err)?; |
There was a problem hiding this comment.
The whole copy is one transaction: begin here, one commit after the loop. The 500-item / 4 MiB bound limits the Rust buffer only. In write_restore_batch the base rows go in one multi-row INSERT, but the loop at line 464 writes secondary-index rows one INSERT per item per index. A million-item table with four GSIs is one transaction with four million round trips, WAL held for the duration, and the target pinned CREATING throughout, which widens the sweep exposure above. Batch the index rows per index per batch, and either commit per batch with cleanup on failure or document the single-transaction cost for large restores.
There was a problem hiding this comment.
Fixed: one commit per batch, one multi-row INSERT per index per batch. The catalog side still holds one REPEATABLE READ snapshot for the copy (so the backup rows are read whole); that is documented in the rustdoc.
| # DescribeTable keeps reporting where the table came from, finished. | ||
| summary = table["RestoreSummary"] | ||
| assert summary["SourceBackupArn"] == backup_arn | ||
| assert summary["RestoreInProgress"] is False |
There was a problem hiding this comment.
DynamoDB does not return RestoreSummary on DescribeTable once the table is ACTIVE (checked at ACTIVE and 30 s later). This assertion pins the ExtendDB behaviour as correct. The test should assert the member is absent after ACTIVE, and the engine should stop emitting it.
There was a problem hiding this comment.
The test now asserts the member is absent once ACTIVE, and the engine no longer emits it.
| | What a restore carries across | Key schema, attribute definitions, items, secondary indexes, billing mode and provisioned throughput, SSE settings. Stream settings, TTL, tags, auto scaling, IAM policies, and CloudWatch settings must be reapplied by hand | The same set on PostgreSQL, SQLite, and MongoDB, plus table class and on-demand throughput. Streams, TTL, tags, and deletion protection are not restored. A backup written by a version before catalog 0.0.4 carries no table definition and restores as keys and items only, with 5/5 provisioned throughput | | ||
| | `RestoreTableFromBackup` overrides | Current request members are `BillingModeOverride`, `ProvisionedThroughputOverride`, `GlobalSecondaryIndexOverride`, `LocalSecondaryIndexOverride`, `SSESpecificationOverride`, `OnDemandThroughputOverride`, and `VectorIndexOverride`; DynamoDB applies them to the restored table | PostgreSQL, SQLite, and MongoDB support `BillingModeOverride` and `ProvisionedThroughputOverride`. Switching to `PAY_PER_REQUEST` drops table and GSI throughput. `GlobalSecondaryIndexOverride`, `LocalSecondaryIndexOverride`, `SSESpecificationOverride`, `OnDemandThroughputOverride`, and `VectorIndexOverride` are refused with `ValidationException` before the target table is created. Switching an on-demand backup with GSIs to `PROVISIONED` is likewise refused because it requires `GlobalSecondaryIndexOverride` | | ||
| | Multi-part base table keys (preview) | Not supported | A backup of a table created with `enable_multipart_keys` is refused at restore with `ValidationException`; the item paths address such tables by their first HASH and RANGE attribute only | | ||
| | `DeleteBackup` during a restore from that backup | `BackupInUseException` | `BackupInUseException` (HTTP 400) until the restore completes or fails, on all three backends. PostgreSQL and SQLite register the restore in the same transaction that creates its target, with the backup row held so a concurrent `DeleteBackup` orders strictly before (the restore then sees no backup) or after (it sees the restore). MongoDB has no cross-collection transaction and uses a claim marker on the backup's metadata instead: a restore claims the backup before reading it and releases the claim once its `CREATING` target exists; `DeleteBackup` claims it the same way before dropping data. A claim left by a crashed process is cleared at the next startup, so a backup is never stuck undeletable; until then a restore or delete of that backup gets `BackupInUseException` | |
There was a problem hiding this comment.
"A backup is never stuck undeletable" is false in two cases on this branch: a MongoDB crash during the base $out copy leaves a CREATING target that blocks DeleteBackup with BackupInUse until the catalog is hand-edited, and on SQLite a restore whose client disconnected leaves the same state until restart. The next row's "PostgreSQL, SQLite, and MongoDB match" is only observable on MongoDB: on the SQL backends the restore completes inside the request, so a sequential client's DeleteTable or DeleteBackup after the response succeeds every time.
There was a problem hiding this comment.
Rewritten for four backends. Rows that rest on code reading or storage-level tests are marked (from the code), numbers from earlier builds of the branch are marked (earlier build), and the header says so.
| | Restore interrupted by a server crash | Managed by the service | PostgreSQL removes an abandoned `CREATING` restore target on a control-plane pass; SQLite removes one when the next server starts. MongoDB has no abandoned-restore sweep: the target remains `CREATING`, the API continues to refuse `DeleteTable`, and an operator must remove its catalog and data collections by hand | | ||
| | Point-in-time recovery | 35-day window | Not supported; `UpdateContinuousBackups` and `RestoreTableToPointInTime` are refused | | ||
| | Where backups live | Managed storage, independent of the table | Inside the database the server already uses, so a lost database loses its backups too: on PostgreSQL in the catalog database (`backups`, `backup_items`, `backup_definitions`, `table_restores`), on SQLite in the one database file, on MongoDB as a `backups` metadata collection in the catalog database and one data collection per backup in the data database. Take database-level backups for off-host copies until the backup store program ships | | ||
| | Backup creation status | `CreateBackup` returns while the backup is `CREATING`; `ListBackups` and `DescribeBackup` expose that state until it becomes `AVAILABLE` | SQLite similarly exposes `CREATING` while it copies: size and item count remain zero, `DeleteBackup` is refused, and `RestoreTableFromBackup` reports the backup missing. PostgreSQL and MongoDB complete their copy before returning and expose only `AVAILABLE` | |
There was a problem hiding this comment.
"SQLite similarly exposes CREATING while it copies" reads as if the caller gets a CREATING response. The caller blocks until AVAILABLE on all three backends (1.4 s for 50,000 items; minutes for a large table), and only a concurrent observer sees the CREATING row. Say that, and note that CreateBackup blocks for the copy duration, or return CREATING and let the detached task finish, which the SQLite code already supports.
There was a problem hiding this comment.
Rewritten: CreateBackup blocks the caller for the copy on all four backends; only a concurrent observer sees CREATING. Detaching CreateBackup is left for a follow-up.
| Upgrade sequence, on PostgreSQL and SQLite alike: | ||
|
|
||
| ```bash | ||
| extenddb stop --config extenddb.toml |
There was a problem hiding this comment.
Followed literally on a 0.0.3 deployment this fails: migrate connects as a PostgreSQL admin and the manual gives no credentials, so it falls back to the OS user and prints Error: Internal("Internal(\"Cannot connect as admin: ... password authentication failed for user \"lhnng\"\")"). Show --pg-user and EXTENDDB_PG_PASSWORD here, and flatten that nested error in the tool. The preview step on line 203 also exits 1 with Error: --yes is required, so a scripted upgrade under set -e stops at the preview; exit 0 for the preview or say the non-zero exit is intended. Nothing here tells a SQLite operator about the backup_items rebuild, and the backup and rollback steps above hardcode extenddb_catalog, extenddb, and OWNER extenddb.
There was a problem hiding this comment.
--pg-user and EXTENDDB_PG_PASSWORD are shown; the error is one plain line naming the role it tried; the preview exits 3 when work is pending and 0 when up to date (documented); the commands are parameterized; and there are SQLite, Cassandra and MongoDB subsections.
|
|
||
| CREATE INDEX IF NOT EXISTS idx_table_restores_backup ON table_restores (source_backup_arn); | ||
|
|
||
| UPDATE settings SET value = '0.0.4' WHERE key = 'catalog_version'; |
There was a problem hiding this comment.
#368 and #335 also ship a 003_*.sql that writes 0.0.4. The ledger keys on filename and converge_catalog_version compares numbers, so if #368 merges first and stamps 0.0.4, this binary applies its own 003 and converges with no refusal: two different schemas both called 0.0.4, and the startup gate cannot tell them apart. Whoever merges second renumbers to 004 and 0.0.5 (plus the two assertions in the migrations.rs tests and the manual's 0.0.4 section). The order should be agreed before any of the three lands.
There was a problem hiding this comment.
Not renumbered; I need your call on the order (open question in the PR body: #396 assumes this PR keeps 003/0.0.4 and #335 moves to 004/0.0.5, and does not place #368). Meanwhile migrate applies an unrecorded catalog migration file even when the version already matches, and PostgreSQL serve refuses to start on a catalog missing one of its files, so a second 003 can no longer be skipped silently.
ba9f869 to
8dab086
Compare
LeeroyHannigan
left a comment
LeeroyHannigan
left a comment
There was a problem hiding this comment.
I re-ran the three items from the last round and the four changes in this revision on all four backends and against the live service. CodeQL is green, the marker is gone, and the body sentence is right. During a catalog failure an item operation on an existing table now answers InternalServerError instead of ResourceNotFoundException, ListBackups pages across a server.region change, the name-before-backup order matches the service on every input I sent, and the region warning is in the banner.
The branch conflicts with main. The four marked hunks are signature and placement. The rebase also needs two changes that git does not flag.
- The
AuthorizationStoretest double inrequest_helpers.rsneedsfetch_user_created_atandfetch_role_created_atfrom #420, or the merged tree does not compile. - The backup branch of
evaluate_request_policiespassesRequestParams::default(). After #420 every other action evaluatesaws:RequestedRegion,aws:SecureTransport,aws:SourceIpandaws:UserAgentfrom the request, and the five backup actions would evaluate them as absent. What I ran is in the inline comment onrequest_helpers.rs.
The differences doc gained a sentence in this revision that is wrong about the service, also flagged inline.
The override refusal and the error texts stay tracked separately, as before. One more text goes on that list. A different backup restored into a name that is a CREATING restore target gets TableInUseException on both, and the service's text is Attempt to change a resource which is still in use: Table: <name> is being used.
| &action, | ||
| &resource_arn, | ||
| false, | ||
| extenddb_auth::policy::context::RequestParams::default(), |
There was a problem hiding this comment.
After a rebase onto main, this branch is the only one that evaluates no request context. #420 fills region, secure_transport, source_ip and user_agent into RequestParams in the batch branch above and the generic path below. This branch keeps default(), so DescribeBackup, DeleteBackup, RestoreTableFromBackup, ListBackups and CreateBackup evaluate aws:SourceIp, aws:SecureTransport, aws:RequestedRegion and aws:UserAgent as absent.
I merged main locally and ran an Allow of dynamodb:* on * with IpAddress aws:SourceIp 10.0.0.0/8. GetItem from 10.1.1.1 was allowed and DescribeBackup from 10.1.1.1 was AccessDeniedException. With the same four fields filled here, both were allowed. The backup_authz_tests pass None, None, false, so they do not reach this path; a test that puts one of the four keys on a backup action would.
There was a problem hiding this comment.
Fixed in 7963024. evaluate_request_policies now takes a RequestContext (region, TLS, peer address, User-Agent) and every branch builds its RequestParams from it, the backup branch included, so the five backup actions evaluate aws:SourceIp, aws:SecureTransport, aws:RequestedRegion and aws:UserAgent like any other request. New test backup_operations_evaluate_the_request_level_condition_keys: an Allow of dynamodb:* on * conditioned on each of the four keys allows DescribeBackup, DeleteBackup, ListBackups, CreateBackup, RestoreTableFromBackup and GetItem from 10.1.1.1, and an IpAddress 10.0.0.0/8 Allow denies all of them from 192.0.2.1. With RequestParams::default() put back on the backup branch, the test fails. The admin guide now says these keys apply to backup evaluations.
| | Members of the `RestoreTableFromBackup` response | The `CREATING` description omits `DeletionProtectionEnabled`, the table's `ProvisionedThroughput.NumberOfDecreasesToday`, and each GSI's `IndexStatus` | The same on all four backends; `DescribeTable` reports the first two. Each GSI's `ProvisionedThroughput` keeps `NumberOfDecreasesToday`; what the service sends there was not measured | | ||
| | Restoring into a name that exists | `TableAlreadyExistsException`; into a name that is itself a `CREATING` restore target, `TableInUseException` "Table: <name> is already being restored from backup: <arn>" (measured) | `TableAlreadyExistsException` on all four backends for an existing table; `TableInUseException` with the service's text, naming the backup the existing target is being restored from, for a name that is a `CREATING` restore target (the backends tell the two apart inside the registration transaction or lightweight transaction, so two simultaneous requests cannot both pass). The existing table or target is unchanged | | ||
| | `UpdateTable` on a `CREATING` restore target | `ResourceNotFoundException` (measured on DynamoDB) | `ResourceInUseException` on all four backends, the engine's mapping of a table that is not `ACTIVE`. A plain `CreateTable` into the target's name is `ResourceInUseException` "Table already exists: <name>", the service's text for a taken name (measured on DynamoDB), without the backup ARN. The target is unchanged | | ||
| | `BackupArn` that is malformed or belongs to another account | Not an ARN of the right shape, too long (over 1024), a control character, a slash or `..` in the region: `ValidationException` "Invalid Request: BackupArn is not valid". The right shape with an id that is not a backup id (wrong length, not hex, a wildcard): `ValidationException` "Invalid Backup ARN" (both measured; which input gets which text was not captured). Surrounding whitespace, a trailing newline for one, is trimmed, and an ARN naming another region of the caller's account resolves (measured). One in another partition is `ValidationException` (measured; text not captured) | The same two texts on all four backends, split by ExtendDB's parser: a string that is not `arn:aws:dynamodb:<region>:<account>:table/<name>/backup/<id>` with a region in `[a-z0-9-]`, a digit-only account, a name in `[a-zA-Z0-9_.-]`, only printable ASCII, and at most 1024 bytes after trimming is "Invalid Request: BackupArn is not valid" (so is one in a partition other than `aws`, the only one ExtendDB issues); one of that shape whose id is neither `[0-9]{14}-[0-9a-f]{8}` nor a shape an earlier build issued (13 digits, `-`, 8 hex digits, which every backend issued before the shared id; 13 bare digits, which PostgreSQL v0.1.0 to v0.1.2 issued) is "Invalid Backup ARN"; no build has put a UUID in a backup ARN, so none resolves. Surrounding ASCII whitespace is trimmed (only a trailing newline was measured on the service; trimming all surrounding whitespace is ExtendDB's generalisation), and the region is checked for shape only: the backup is looked up under the ARN rebuilt in this deployment's `server.region` first and, when that is not found and the caller wrote a different ARN, under the ARN as written, so another region's resolves and a backup created under an earlier `server.region` stays reachable by the ARN `ListBackups` reports for it, though not by that ARN rewritten with the new region (see the upgrade manual on `server.region`). A bare 13-digit id (the shape PostgreSQL v0.1.0 to v0.1.2 issued) passes the id check, so such an ARN that names no backup is `BackupNotFoundException`, where the service answers "Invalid Backup ARN". A `server.region` outside `[a-z0-9-]` makes the server's own ARNs "not valid"; the server starts and logs a warning saying so. A well-formed ARN naming another account, of any number of digits, is `AccessDeniedException` on `DescribeBackup`, `DeleteBackup`, and `RestoreTableFromBackup` (not measured on the service). The same checks apply to `ListBackups`'s `ExclusiveStartBackupArn`, where another account's or region's cursor is instead a `ValidationException` (see `ListBackups` paging). Authorization evaluates an ARN the parser refuses, or one in another partition, against `table/*`, so a caller allowed on `table/*` gets the engine's `ValidationException`, as in dev mode, and a caller whose policy names only backup ARNs gets `AccessDeniedException`; the service's order for that case was not measured. Same on all four backends | |
There was a problem hiding this comment.
The new sentence says the service answers Invalid Backup ARN for a bare 13-digit id. I sent DescribeBackup with .../backup/1234567890123 to the service and got ValidationException with Invalid Request: BackupArn is not valid.
There was a problem hiding this comment.
Corrected: the row now says the service answers ValidationException "Invalid Request: BackupArn is not valid" for a bare 13-digit id. I also corrected the "Restoring into a name that exists" row with the different-backup TableInUseException text you measured: it gives both service texts and says ExtendDB always uses the same-backup one. Changing ExtendDB's text is on your list.
79c66b9 to
7963024
Compare
|
Rebased onto main (ba53851) as 7963024. It's still one commit and mergeable.
Testing on the rebased head:
|
LeeroyHannigan
left a comment
LeeroyHannigan
left a comment
There was a problem hiding this comment.
Thanks for the work on this @robinnsc
🚀
7963024 to
a92bf5a
Compare
|
Rebased onto main (b5bffa3, v0.1.14) as a92bf5a, still one commit. Main's #418, #419 and #421 overlap this PR's authorization and key-info work, so main's code is taken there:
Gates on a92bf5a:
|
a92bf5a to
077e359
Compare
|
077e359: CodeQL flagged two |
…; recover abandoned restores
Backup and restore now reproduce the table that was backed up, behave the
same on PostgreSQL, SQLite, MongoDB, and Cassandra, and match DynamoDB on
the points measured against the service.
Engine. The engine resolves a backup into one validated CreateTableInput
before any backend sees it: it runs the multi-part-key and vector-index
refusals, applies the overrides, and runs validate_create_table. Backends
only create the target, register the restore, and copy. The trait gains
backup_restore_source; restore_table_from_backup takes a resolved
RestoreRequest; restore_table_to_point_in_time is removed. Once the request
is authenticated and authorized, the whole restore runs in a task of its
own, so a client that disconnects does not cancel it part-way.
Restore runs after the response on every backend: the target is returned
CREATING and copied by a background task. While it is CREATING, DeleteTable
on it is ResourceInUseException and DeleteBackup on its source is
BackupInUseException. A target whose server stopped mid-copy is removed and
logged: PostgreSQL after 60 s, SQLite at the next start, MongoDB and
Cassandra by a heartbeat sweep at startup and every 15 s.
Wire behaviour: RestoreDateTime is the backup's creation time and
SourceTableArn is set; RestoreSummary appears only while CREATING, on every
table description (TableDescription::prepare_for_response). DescribeBackup
reports SourceTableDetails.ProvisionedThroughput and
SourceTableFeatureDetails.
ListBackups pages with Limit and LastEvaluatedBackupArn and carries TableId.
Restoring into an existing name is TableAlreadyExistsException; a malformed
BackupArn is ValidationException, checked before the target name.
SSESpecificationOverride, GlobalSecondaryIndexOverride,
LocalSecondaryIndexOverride, and OnDemandThroughputOverride are applied as
the service applies them (an empty list restores no indexes of that kind, a
subset restores those, a GSI entry's throughput is the index's); an unknown
or changed index definition, a throughput member contradicting the billing
mode, {0, 0}, and BillingModeOverride=PROVISIONED with a GSI whose
throughput no entry supplies are ValidationException with the service's
text. TableClass is not carried across; the target is STANDARD. ListBackups
filters on BackupType and the time range; its cursor must match the
account, region, and TableName filter (a listed backup's stored ARN in an
earlier region is accepted, so paging passes it), and the time bounds are
checked. A BackupArn in another partition is a ValidationException. BackupArn and
ExclusiveStartBackupArn shapes are checked as the service checks them
("Invalid Request: BackupArn is not valid" / "Invalid Backup ARN"),
surrounding whitespace is trimmed, and an ARN of the caller's account in
another region resolves (looked up in the server's region first, then as
written, so backups created under an earlier server.region stay
reachable); the 13-digit ids of earlier builds still resolve. BackupName
and TableName are validated as the model says (3 to 255 characters,
[a-zA-Z0-9_.-]+) with the service's "validation error(s) detected" texts;
TableName may be the table's ARN; CreateBackup of a missing or CREATING
table is TableNotFoundException. DescribeBackup propagates a failure to
read the definition as an internal error and degrades only for a version
it does not know; DeleteBackup returns the full description DELETED.
Restoring into a CREATING restore target is TableInUseException and a
second restore of a backup whose restore is CREATING is
BackupInUseException. Each backend checks the target name before the
backup and before a restore slot is taken; the target insert and the
registration transaction, claim, or lightweight transaction are the
fences. The key-info cache stores
no negative entry for a name the catalog has in a non-ACTIVE state, so the
first read after ACTIVE succeeds; authorization's key-info snapshot (#418)
records such a name as missing, so the engine does not look it up again. A
PROVISIONED backup whose GSI has no throughput restores with the table's
throughput. CreateTable and UpdateTable refuse GSI throughput below 1 on
PROVISIONED tables and more than 100 projected NonKeyAttributes.
CreateBackup of a table with vector indexes is refused. Backup ids are 14
zero-padded millisecond digits on every backend.
Authorization: backup operations use main's resource authorization (#419).
A BackupArn is trimmed of surrounding ASCII whitespace before it is
canonicalized, as the engine trims it before resolving the backup, so a
Deny on a backup ARN cannot be bypassed with a trailing newline.
PostgreSQL: the copy commits per batch with one multi-row INSERT per index;
the lock session is heartbeated; at most 4 restores per account and
min(16, pool_size / 2) per server, then LimitExceededException after 30 s,
with the whole restore, admission included, in a task the request waits
on; only SQLSTATE 53300 on the lock connection is LimitExceeded. The backup
row is locked FOR UPDATE while a restore registers. serve refuses a
catalog that does not record one of the binary's migration files, and
converges a pre-release 0.0.4 catalog shape with a 5 s lock timeout.
migrate uses the data database name init recorded and accepts --data-db.
SQLite: serve refuses a pre-release 0.0.4 shape; the backup_items rebuild is
named in the migrate preview and logged; the pool_size clamp is logged.
CreateBackup is admitted against the pool (pool_size / 2 - 1 at once, then
LimitExceededException after 30 s). A GSI added by UpdateTable with
throughput on a PROVISIONED table no longer makes DescribeTable fail, and
rows earlier builds wrote are read.
MongoDB: twenty concurrent restores no longer wedge the pool (the
registration hand-over held a checked-out connection while waiting for
another; both futures are polled together now); restores are admitted per
account (4) and per server (min(16, pool / 2)), LimitExceededException
after 30 s, and a registration longer than 60 s answers an internal error
while the restore continues. UpdateTable on a CREATING restore target is
refused. DeleteBackup and the sweep never release a delete claim once the
data drop may have started; a held delete claim answers BackupInUse; a
concurrent restore of one backup retries for 2 s, then BackupInUse; catalog
reads are pinned to the primary with read concern local and writes to
w: majority, and a weaker write concern in the connection string is
refused at startup; heartbeats refresh at once. The test-only protocol
model covers two restores, the delete and its stop points, and the sweep's
delete takeover.
Cassandra: migration V005 (catalog 0.0.4) stores backup definitions and
restore registrations (registered with a lightweight transaction that
requires the backup AVAILABLE and unclaimed); the copy uses the normal
put path with GSIs written
synchronously; ordinary CreateTable does not name the V005 columns, so an
unmigrated catalog breaks only CreateBackup, RestoreTableFromBackup, and
DeleteBackup. The restore heartbeat stops on every exit of the copy. On an
unmigrated catalog the backup operations are a ValidationException naming
`extenddb migrate --yes`. UpdateTimeToLive on a table with an asynchronous
GSI is a ValidationException instead of a 500. init writes the
application credentials into the config, created with mode 0600. A
pre-existing defect is fixed: an on-demand GSI omitted
ProvisionedThroughput.
Catalog guards fail closed on an unparseable version. extenddb migrate:
the preview exits 3 when work is pending (main's resource-tag rewrite
included) and 0 when up to date; unrecorded
catalog migration files are pending; admin connection errors are one
plain line; --cassandra-user/--cassandra-pass and
EXTENDDB_CASSANDRA_PASSWORD are accepted.
Docs: differences-from-dynamodb, dynamodb-limits, the admin, upgrade, and
architecture manuals, and the MongoDB design doc are updated for four
backends.
Tests: tests/test_backup_restore_fidelity.py covers the restore surface on
all four backends; devtools/run-cassandra-tests runs its large-backup tests
with 10,000 items, because 60,000 made cdrs-tokio drop its only
connection. tests/test_ttl.py waits for ENABLING to end before disabling.
Signed-off-by: Scott Robinson <robinnsc@amazon.com>
077e359 to
3732655
Compare
Conflicted files and how each was resolved: - crates/storage-postgres/migrations/003_control_chars_encoding.sql: renamed to 004_control_chars_encoding.sql. Main's #384 added 003_backup_definitions.sql (catalog 0.0.4), so the encoding marker becomes migration 004 and moves the catalog to 0.0.5. The data code migration keeps its name, 004_escape_control_chars: main added no data migration since the base, and the data ledger is separate from the catalog ledger. - crates/storage-postgres/src/migrations.rs: CATALOG_MIGRATIONS lists both 003_backup_definitions.sql and 004_control_chars_encoding.sql; the count and version tripwire now expects 4 files and 0.0.5. Main's runner changes (version convergence after the walk, pending catalog files, shape steps) and this branch's 004_escape_control_chars code both kept. - crates/storage-postgres/src/lib.rs: CATALOG_VERSION is 0.0.5. Both check_data_migrations_applied (this branch) and prepare_catalog_for_serve (main) kept; the serve path runs the catalog gate first, then the data migration check. - crates/storage-postgres/src/data/index.rs: main's multi-row insert_index_rows_multi kept, with the projected item serialized through item_to_json so index rows carry the escaped form. - crates/storage/src/util/mod.rs: both sides' modules and re-exports kept (backup_id, catalog_version, control_chars). - tests/test_cli_vector_catalog_migration.py: main's structure kept; ENCODING_MIGRATION is 004_control_chars_encoding.sql, the current version is 0.0.5 and the future version 0.0.6; the rollback to 0.0.2 deletes all three later ledger rows. The replay test deletes only the vector row, as on main, since the runner converges the version after the walk. - docs/manuals/07-upgrade-manual.md: Catalog 0.0.5 (Current) is this branch's entry, PostgreSQL only, placed above main's Catalog 0.0.4 entry; the Current Status names 0.0.5 on PostgreSQL and 0.0.4 on SQLite and Cassandra; the migration list and the Data Migrations section name the new numbers. - docs/getting-started.md, docs/manuals/01-architecture-guide.md, docs/manuals/04-quickstart-setup-guide.md: main's 0.1.14 sample output with catalog 0.0.5 for PostgreSQL; the architecture guide keeps SQLite and Cassandra at 0.0.4 with the doc guard marker. Follow-on edits for the version bump: sample output in the install manuals and the admin guide reads 0.0.5; three rows in differences-from-dynamodb.md that name catalog 0.0.4 as the version that introduced a behaviour carry the doc guard marker; catalog_converge.rs builds its scratch catalog through 004 and records 004_escape_control_chars in the data ledger, the way init leaves a deployment.
…m the decoded item Main's restore (#384) reads each backup_items document as text, parses it into an Item and writes the text into the target's item_data unchanged. On this branch that text carries the control-character escape, and the key columns and index rows are built by helpers that escape their input, so the parsed item was escaped a second time: a restored table whose items hold U+0000 or U+0001 got key columns and index rows in a form the read path does not produce. The item is now decoded with json_to_item before the key helpers see it, and the key helpers are the escaping ones from key_text, so every column is escaped exactly once; the item_data text is still copied as it is. The backup copy sizes each item from the decoded form for the same reason. The parallel Scan cross-segment check (#203) hashes the candidate key's text and compares it with the hash of the stored pk column; it now uses the escaping pk_to_text so both sides hash the same text for keys with control characters. A storage-level test restores a table whose keys, index keys, attribute names and nested values hold U+0000 and U+0001 and compares the stored rows of the base table and both index tables with the source's, then reads each item back.
What
Backup and restore now behave the same way on all four storage backends (PostgreSQL, SQLite, MongoDB, Cassandra), and closer to DynamoDB, on the points the round-2 and round-4 reviews measured against the service.
CreateTableInputbefore any backend sees it. It runs the multi-part-key and vector-index refusals, applies the overrides, and runsvalidate_create_tableon the result, for every backend. Backends now only create the target, register the restore, and copy. Cassandra (Feat/storage cassandra in tree #338) previously skipped the refusals and dropped the secondary indexes.RestoreTableFromBackupreturns the targetCREATINGand copies in a background task on all four backends, so a client that times out or disconnects no longer loses the restore or locks the name and the backup. Once the request has been authenticated and authorized, the engine runs the rest of it (reading the backup, validation, registering the restore) in a task of its own, so a client that disconnects before the response no longer cancels it part-way. A disconnect while the request is still being authenticated or authorized cancels it with nothing created (see Not changed here).CREATING,DeleteTableon it isResourceInUseExceptionandDeleteBackupon its source isBackupInUseException. A target whose server stopped mid-copy is removed without operator action, and each removal is logged with the table name:RestoreSummarymatches the service.RestoreDateTimeis the backup's creation time andSourceTableArnis set. No response that describes a table (CreateTable,UpdateTable,DeleteTable,DescribeTable, the restore response) carriesRestoreSummaryunless the table isCREATING; one rule inTableDescription::prepare_for_responseapplies it to all of them.DescribeBackupreportsSourceTableDetails.ProvisionedThroughput, which is required in the shape and{0, 0}for on-demand. It also reportsSourceTableFeatureDetails: GSIs, LSIs, TTL, stream, and SSE. A definition version this build does not know is described without feature details; any other failure to read the definition (a row that does not decode, a lost connection) is an internal error, as it is forRestoreTableFromBackup, instead of a description that hides it.DeleteBackupreturns the fullDescribeBackupdescription withBackupStatusDELETED, includingSourceTableDetails.ProvisionedThroughputandSourceTableFeatureDetails. It alone tolerates a definition that cannot be read (it answers{0, 0}and no feature details, and logs a warning), so a damaged backup stays deletable; this is ExtendDB's choice, stated in the docs.ListBackupshonoursLimit(1 to 100, default 100) andExclusiveStartBackupArn, and returnsLastEvaluatedBackupArn. Summaries carryTableId. It filters onBackupType(USERwhen absent, every type forALL; every ExtendDB backup isUSER) and onTimeRangeLowerBound(inclusive) andTimeRangeUpperBound(exclusive), before paging. The cursor is checked as the service checks it (measured on DynamoDB): one from another account isValidationException"Invalid Request: The supplied ExclusiveStartBackupArn does not match the account", one from another region or another table than theTableNamefilter is refused with the same shape of text, a time bound before the epoch is refused, and a lower bound above the upper bound is "Time range lower bound must be less than or equal to upper bound".BackupNameandTableNameonCreateBackup, andTableNameonListBackups, are validated as the service model says: 3 to 255 characters and[a-zA-Z0-9_.-]+, with the service's "1 validation error detected: Value '' at 'backupName' failed to satisfy constraint: ..." texts, joined under "2 validation errors detected:" when both fail (the order of the two clauses was not captured; the length clause comes first).TableNametakes the deployment's configured table-name limits. Before this a 200 KBBackupNamewas stored and returned in everyListBackupspage (a 500 on Cassandra) and a NUL byte was a 500 on PostgreSQL.CreateBackupacceptsTableNameas the table's ARN (measured on DynamoDB; botocore refuses the ARN form client-side, so only a hand-built request sends it);CreateBackupon a table that does not exist or is stillCREATINGisTableNotFoundException"Table not found: " (measured) on all four backends.TableAlreadyExistsException; into a name that is itself aCREATINGrestore target,TableInUseException(measured on DynamoDB); the text is the service's "Table: is already being restored from backup: " whichever backup is restored, where the service answers "Attempt to change a resource which is still in use: Table: is being used." for a different backup. A secondRestoreTableFromBackupof a backup while a restore from it is stillCREATINGisBackupInUseException(measured). Both are decided inside the registration transaction (PostgreSQL, with the backup row lockedFOR UPDATE; SQLite), the claim (MongoDB), or the lightweight transaction (Cassandra), so two simultaneous requests cannot both pass; the name is checked before the backup and before a restore slot is taken, so the same backup restored again into its ownCREATINGtarget's name isTableInUseExceptionat once on all four, also while the account's restore slots are all in use. A restore of a backup whoseDeleteBackupis in progress answersBackupInUseExceptionon MongoDB andBackupNotFoundExceptionon the other three (documented; the service's answer was not measured). A plainCreateTableinto aCREATINGrestore target staysResourceInUseException"Table already exists: ".BackupArnandExclusiveStartBackupArnare checked as the service checks them (measured on DynamoDB): a string that is notarn:aws:dynamodb:<region>:<account>:table/<name>/backup/<id>with a region in[a-z0-9-], a digit-only account, a name in[a-zA-Z0-9_.-], over 1024 bytes, or carrying a control or non-ASCII byte isValidationException"Invalid Request: BackupArn is not valid"; a well-shaped ARN whose id is not a backup id is "Invalid Backup ARN" (which text goes with which input was captured for one input only, a bare 13-digit id answered "not valid"; the rest of the pairing is ExtendDB's). Surrounding ASCII whitespace is trimmed (the service was measured trimming a trailing newline). An ARN of the caller's account in another region resolves: the engine looks the backup up under the ARN rebuilt in the server's region first and, when the ARN as written differs, under that spelling, so backups created under an earlierserver.regionstay reachable by the ARNListBackupsreports (not by that ARN rewritten with the new region), andListBackupsaccepts such a backup's stored ARN asExclusiveStartBackupArn, so paging passes it. Aserver.regionoutside[a-z0-9-]logs a startup warning naming the consequence. The ARN is checked before the target name; another account's isAccessDeniedException; another partition's isValidationException. Before this, every malformed shape resolved toBackupNotFoundExceptionand a NUL byte was a 500 on PostgreSQL.SSESpecificationOverrideis applied.GlobalSecondaryIndexOverride,LocalSecondaryIndexOverride, andOnDemandThroughputOverrideare applied as the service applies them (measured on DynamoDB): an empty list restores no indexes of that kind, a subset restores those indexes and prunes the attribute definitions only they used, a GSI entry'sProvisionedThroughputis the restored index's throughput, a one-member or changedOnDemandThroughputOverrideis the restored table's on-demand throughput (including aPROVISIONEDbackup restored as on-demand), and the on-demand-to-PROVISIONEDroute works with per-index throughput in the list. Refused as on the service: an unknown or changed index definition (key schema or projection), a throughput member contradicting the billing mode,{0, 0}, and aPROVISIONEDrestore of a GSI without throughput, with the service's text "Invalid Request: Must override ProvisionedThroughput if BillingMode is overridden to PROVISIONED for index ": that refusal fires wheneverBillingModeOverride=PROVISIONEDis sent with a GSI and no list entry supplies its throughput, whatever the backup's billing mode or recorded throughput (Lee's measurement was on an on-demand backup; thePROVISIONED-backup case follows the service's own message and was not measured), and when a list entry omits throughput on aPROVISIONEDrestore without the override (text not measured). A GSI entry'sOnDemandThroughputis accepted and not recorded (ExtendDB keeps no per-index on-demand maximum).NonKeyAttributesare compared as a set.TableClassis no longer carried by a restore: the target isSTANDARD, as on the service (measured). The backup definition keeps the class.RestoreTableFromBackupresponse no longer carriesDeletionProtectionEnabledor the table'sProvisionedThroughput.NumberOfDecreasesToday, which the service omits there; a GSI'sNumberOfDecreasesTodayis kept (not measured).PROVISIONEDnow says what works: restore withPAY_PER_REQUEST, then switch withUpdateTable.PROVISIONEDbackup whose GSI has no provisioned throughput restores, and that GSI takes the table's throughput (orProvisionedThroughputOverride's). ExtendDB'sCreateTableaccepts such a table andUpdateTabletoPROVISIONEDproduces one; the service cannot produce such a backup, so the choice is ExtendDB's. The earlier restore-only stopgap that refused these backups is removed.CreateTableandUpdateTablenow apply the two checks the stopgap applied only at restore: a GSIProvisionedThroughputbelow 1 on aPROVISIONEDtable (and onUpdateTable's GSICreate) isValidationException, and so are more than 100 projectedNonKeyAttributesacross a table's GSIs and LSIs. Restore runs the same checks throughvalidate_create_table.CreateBackupof a table with vector indexes is refused up front. It no longer returns an AVAILABLE backup that can never be restored.BackupCreationDateTimeis that millisecond. The ids earlier builds issued (13 digits plus the hex suffix from v0.1.3 to v0.1.14; bare 13 digits from PostgreSQL v0.1.0 to v0.1.2) still resolve; they carry no time, so one that has been deleted or filtered out cannot serve as aListBackupscursor.DescribeTablereportsACTIVEsucceeds (measured on DynamoDB). The server's key-info cache no longer stores a negative entry for a name the catalog has in a non-ACTIVEstate: on a not-found answer it asks the catalog once more withDescribeTableand caches nothing. Authorization (main's fix(auth): enforce dynamodb:LeadingKeys on Query, batch and transaction operations #418) records such a name as missing in the key-info snapshot it hands the engine, and fails closed on any other lookup error, so a request against aCREATINGname costs onetable_key_infoand onedescribe_tableload (measured on SQLite with this PR's earlier hand-off, which fix(auth): enforce dynamodb:LeadingKeys on Query, batch and transaction operations #418's snapshot replaces: 40 and 41 catalog loads for 40 refusedGetItems). Before this a read during the copy poisoned the cache and reads failed for up to 5 s afterACTIVE;CreateTabletargets had the same lag and get the same fix.pool_sizein total (capped at 16; 10 with the defaultpool_size). A restore that cannot get both within 30 s isLimitExceededException. The whole restore, admission included, runs in a task the request only waits on, so a client that disconnects during the wait does not cancel it; the account slots belong to the engine, not the process.extenddb migratefinds the data database through the nameinitrecorded in the catalog'ssettings, so a deployment initialised with--data-db <other-name>migrates without further flags, and accepts--data-dbto override it. Before, it derived the name from the catalog database name and failed with "database does not exist".LimitExceededExceptiononly when PostgreSQL refuses it with too_many_connections (SQLSTATE 53300); any other failure is an internal error.serverefuses a catalog whoseschema_historylacks one of the binary's catalog migration files; the message names the files andextenddb migrate --yes. The stored version alone does not show this case, because another migration of the same number can have stamped the same version.table_restores.source_table_arn) gets the column atservestartup, with a warning. That step waits at most 5 s for the table lock (lock_timeout) and otherwise refuses to start with the migrate advice.serveno longer drops the pre-releaserestore_date_timedefault, which a still-running earlier-build server relies on; onlymigrate --yesdoes.restore_table_to_point_in_timeis removed from the trait and from every backend.serverefuses to start a file database whose tables are in a pre-release 0.0.4 shape, and names the step andextenddb migrate --yes. The upgrade'sbackup_itemsrebuild is named in the preview with its row count and logged at start and finish. Thepool_sizeclamp is logged.CreateBackupis admitted against the pool: at mostpool_size / 2 - 1backups at once (4 with the default of 10; each holds its reader and its writer),LimitExceededExceptionafter a 30 s wait; a refused backup writes nothing. Before this, a flood ofCreateBackups took every pooled connection and every other request waited behind them.UpdateTablewithProvisionedThroughputon aPROVISIONEDtable made everyDescribeTableof that table fail (Internal("index pt: missing field NumberOfDecreasesToday")). The index's throughput is now written in the shapeDescribeTablereads, and rows earlier builds wrote are read as the same units withNumberOfDecreasesToday0, so such a table is describable again after the upgrade without touching the database file.DeleteBackup, and the sweep finishing an interrupted delete, no longer release their claim once the backup's data may have been dropped. An error or cancellation after that point leaves the backupAVAILABLEand in use until the sweep finishes the delete; before, it could be leftAVAILABLEwith no data, and a restore of it produced an empty table.DeleteBackupthat finds the backup claimed by another delete isBackupInUseException("Backup is being deleted"), where it wasBackupNotFoundException.local, and write concernmajoritywhatever the connection string says, and the$outcopies of CreateBackup and restore are written withw: majority, so the claim protocol's reads see every write the primary acknowledged and an acknowledged claim survives a failover. A connection string that sets a weaker write concern (w=0, a numeric or taggedw,journal=false) is refused at startup, as a non-primary read preference already was.BackupInUseException. Before, it failed at once withBackupNotFoundException.CREATING, and finishes the delete afterwards.RestoreTableFromBackuprequests no longer wedge the server for every later request (reproduced on the previous build with 20 restores of 20 distinct backups: 6 answered, 14 timed out after 60 s, and every later request timed out for the rest of the run; 20 restores of one backup did not wedge, because one backup's claim serializes them). The registration's hand-over leftcreateCollection's pooled connection checked out while it waited for another connection, and the Rust driver never times a checkout out (waitQueueTimeoutMSis not supported inmongodb3.8.0), so with ten such restores the pool was exhausted for good. Both futures are now polled together. Restores are admitted per account (4) and per server (min(16, pool/2), where the pool is the engine client'smaxPoolSize:max_connections, or the driver default of 10, so 5), withLimitExceededExceptionafter 30 s; a request whose registration takes longer than 60 s answers an internal error while the restore continues andDescribeTablereports its target once it exists. The whole restore, admission included, runs in a task the request only waits on. The one-restore-at-a-time exclusion on a backup is bounded by the heartbeat: a restore that stalls between its claim and its insert for longer than the stale window loses the claim to the sweep and can be joined by a second restore of the backup (documented; PostgreSQL and Cassandra have no such window).UpdateTableon aCREATINGrestore target is refused (ResourceInUseException, as on the other three backends; the service answersResourceNotFoundException). Before, it changed the target mid-restore: billing mode, a third GSI, a stream, and deletion protection were accepted and the finished table was not what the backup described.BackupInUsetexts match the other backends ("Backup is being used to restore table : ", or "... to restore another table: " while the claim is held and the target is not yet known).#[ignore]d;cargo test -p extenddb-storage-mongodb --lib protocol_model -- --include-ignoredruns them, and the module doc says when to. Its doc comment now says thatRReleaseis one atomic step for the release, the lost-claim recheck, and the abort, and lists what is not modelled (release failures, the sweep's pre-takeover target check).ProvisionedThroughput{0, 0}inDescribeTable, and a GSI added byUpdateTablewith throughput reports it.CreateTableno longer names the V005 restore columns, so an unmigrated catalog breaks onlyCreateBackup,RestoreTableFromBackup, andDeleteBackup.ValidationExceptionnamingextenddb migrate --yes, instead of an internal error; a running server notices the migration without a restart.UpdateTimeToLiveenabling TTL on a table with an asynchronously propagated GSI is aValidationExceptionnaming the table, the index, and its delay, instead of an internal error that SDKs retry (the engine now maps a backend's validation refusal in the TTL handlers).extenddb initwrites the application role's username and password into the generated config, soservestarts against aPasswordAuthenticatornode without editing it. The config file is created with mode 0600 instead of being written and then narrowed.backup_status = 'AVAILABLE' AND active_restores = null, so a second restore of a backup whose first restore isCREATINGisBackupInUseExceptionand a restore into aCREATINGrestore target isTableInUseException, as on the other backends. A restore whose registration attempts are all taken by a restore that was just starting answersBackupInUseException("another restore of this backup is starting; retry") instead of "Backup not found".BackupArnis trimmed of surrounding ASCII whitespace before it is canonicalized, as the engine trims it before resolving the backup, so a Deny on a backup ARN cannot be bypassed by sending the ARN with a trailing newline (unit testbackup_arn_with_surrounding_whitespace_authorizes_the_trimmed_backup). The engine resolves aBackupArnof the caller's account under the server's region first and then as written, which is the same table and backup id that authorization evaluated.tests/test_auth_permissions.py::TestBackupAuthorizationcoversListBackupsper table, the restore's seven item actions, and a tagged existing target (anaws:ResourceTagAllow:TableAlreadyExistsException; without the tag:AccessDeniedException) in authenticated mode on all four backends.extenddb migrate) refuse a stored version they cannot parse instead of overwriting it, andserveprints the same refusal line asmigrateinstead of wrapping it as an internal error.extenddb migrate:--yesexits 3 when work is pending and 0 when up to date. It used to exit 1 with an error. This covers main's resource-tag condition-key rewrite (fix(auth): evaluate resource tags only as aws:ResourceTag, matching DynamoDB #421) too, whose detection and apply step now sit behind a smallResourceTagSteptrait so the exit-status tests can run without a catalog; main'stests/test_resource_tag_migration.pyis updated to expect exit 3 for its preview.--yes. It lists such files underdata migrationsascatalog <file>.--pg-userandEXTENDDB_PG_PASSWORD. Before, it readInternal("Internal(\"Cannot connect as admin: ...\")").initanddestroyprint bootstrapper errors the same way.--cassandra-userand--cassandra-pass, and reads the password fromEXTENDDB_CASSANDRA_PASSWORDwhen the flag is absent (so doesinit). Without them it could not migrate a Cassandra deployment that usesPasswordAuthenticator.extenddb-appgains anasync-traitdev-dependency..github/workflows/integration.ymladds--test backup_restoreto the PostgreSQL storage-level test step.devtools/run-cassandra-testsruns the fidelity file's large-backup tests with 10,000 items unlessEXTENDDB_TEST_BACKUP_ITEMSis set (see Not changed here).docs/differences-from-dynamodb.md: the Backup and Restore section, rewritten for four backends, with a seven-member overrides table; rows that rest on the code rather than a running server are marked. Two Secondary Indexes rows for the newCreateTable/UpdateTablechecks. New rows forBackupArnshapes, name validation, the cursor checks, reads on a restore target,UpdateTableon aCREATINGtarget, what a restore carries across, backup contents, and the authorization resources.docs/dynamodb-limits.md: the backup rows; projected attributes are now enforced.docs/technical-debt.md: that limit removed from the unenforced list.docs/manuals/07-upgrade-manual.md: parameterized backup and rollback commands, themigratecredentials and exit codes, and PostgreSQL, SQLite, Cassandra, and MongoDB subsections for 0.0.4 (includingserve's refusal of unrecorded catalog files, the 5 s lock wait, and MongoDB's read and write concern).docs/manuals/05-admin-guide.md:[storage.sqlite]; the PostgreSQL restore limits and the session-mode pooler requirement.docs/manuals/01-architecture-guide.md,docs/design/13-storage-mongodb.md(restore flow; catalog read preference and concern), and the version literals in the getting-started and install guides.Why
Round-2 review by Lee Hannigan, which requested changes. It found four blocking problems:
The review also listed wire differences measured against DynamoDB, PostgreSQL robustness issues, tenancy issues (a process-global restore limit, unvalidated stored definitions, and catalog guards that failed open), authorization of backup operations against
table/*, and the upgrade-path problems in the manual. A further review of this round found the MongoDB delete-claim release, and the shapes thatCreateTableaccepted and restore refused.Round-4 review by Lee Hannigan (sent directly) on the previous revision found 19 further problems, all addressed here: twenty concurrent MongoDB restores wedged the server; a SQLite
CreateBackupflood took the whole pool; MongoDBUpdateTablechanged aCREATINGrestore target;DescribeBackupmasked database failures; reads on a restored table kept failing for seconds afterACTIVE; PostgreSQLmigratecould not find a data database named atinit;BackupName,TableName,BackupArnand theListBackupscursor were not validated as the service validates them;CreateBackupof a missing table, a restore into aCREATINGtarget, and a second restore of a backup gave the wrong errors;DeleteBackupreturned a partial description;TableNameas an ARN was refused; index and on-demand overrides the service applies were refused, and two requests the service refuses were accepted;TableClasswas carried across; and the docs and test comments narrated the review. It also listed pre-existing issues, of which the SQLite GSI-throughput storage shape and the authorization resources ofListBackupsandRestoreTableFromBackupare fixed here (the others are under Not changed here).Open questions for the maintainers:
003_*.sqlthat writes catalog 0.0.4. Whichever merges second has to renumber to004_*.sqland 0.0.5 (and themigrations.rsassertions and the manual's section). The design in docs(backup): design revision for off-host backup and restore [backup/restore 1/4] #396 assumes this PR keeps 003 / 0.0.4 and fix(backup): report point-in-time recovery as unsupported #335 is rebased to 004 / 0.0.5, and reserves 005 / 0.0.6 for a later PR of the series; it does not mention Store U+0000 in strings on PostgreSQL and MongoDB #368. Is that the order you want, and where does Store U+0000 in strings on PostgreSQL and MongoDB #368 go?migratereports an unrecorded catalog migration file as pending even at the current version and applies it, and PostgreSQLserverefuses to start until it has, so after a renumbering the second PR's file is applied rather than skipped. The numbering itself still has to be settled.Testing done
Release binaries were built from the code in this PR, one per backend feature, and served on 127.0.0.1 against PostgreSQL 16, mongo:7 (single-node replica set), and cassandra:4.1 with
PasswordAuthenticator, all on one host. Timings compare backends; they do not predict a networked deployment. The fidelity table below ran on those binaries; the large-backup timings and the further checks after them ran on earlier builds of this branch, as each says.tests/test_backup_restore_fidelity.py,tests/test_ttl.py,tests/test_abac.py, andtests/test_auth_permissions.py, each run in full on release binaries built from this PR's head after its rebase onto main (b5bffa38, v0.1.14), plus main'stests/test_authz_resource_arn.py(Cassandra withindex_propagation_delay_msset to 0, as CI runs it; see #407):test_backup_restore_fidelity.py(37)test_ttl.py(11)test_abac.py(69)test_auth_permissions.py(63)test_authz_resource_arn.py(43)test_abac.pyandtest_authz_resource_arn.pyinclude the tests main's #418 to #421 added. In the full run the threeTestBackupAuthorizationtests failed on every backend because their message check expected the resource to end theAccessDeniedExceptiontext, and main's evaluator now appends the reason ("because no identity-based policy allows ...", "with an explicit deny ..."); with the check loosened to accept a suffix, the class passed 3/3 on all four backends on the same binaries.tests/test_resource_tag_migration.py(main's, SQLite only) passed 2/2 under Python 3.12 once its preview assertion was changed to this PR's contract (exit 3, "run again with --yes to apply"); the host's Python 3.7sqlite3module cannot read the catalog's partial index.Earlier passes in this round found three defects, fixed before the run above:
test_first_read_after_active_succeedsraced theCREATINGtoACTIVEtransition on PostgreSQL (its copy of 60,000 items takes about 1.5 s; the test now accepts a read that succeeds onceDescribeTableconfirmsACTIVE); and SQLite, MongoDB, and Cassandra answeredBackupInUseExceptionwhere the service answersTableInUseExceptionfor a restore of a backup into its ownCREATINGtarget's name, because they checked the backup before the name (PostgreSQL checked the name first). All four now check the name at the storage layer before the backup and before a restore slot is taken; storage-level tests on each pin the same-backup case and a missing backup restored into the name, and on PostgreSQL and MongoDB a retry answered at once while every account slot is held. The engine still reads the backup before the storage call, so at the wire a missing backup isBackupNotFoundExceptionwhatever the name; the storage order matters when a backup is deleted between the engine's read and the registration, and for a retry. One Cassandra pass ran against a node that had accumulated about 1,640 tables over two days of test runs and was pausing over 80 s per young-generation GC (from the node's own log); four of its five failures were copies or teardowns that timed out, the fifth was the ordering defect. The runs above used acassandra:4.1container created the day before them.The large-backup tests use a 60,000-item on-demand backup of a table with only a partition key and no secondary indexes (secondary indexes are covered by the smaller tests; with GSIs on 60,000 items the fixture kept a backend's GSI queue or backfill worker busy for minutes and slowed other modules' GSI tests):
test_restore_response_arrives_before_its_copy_finishes: a client with a 10 s read timeout and no retries gets theCREATINGresponse within the timeout, the target is seenCREATINGwhile it is polled, and it reachesACTIVEwith every item. It shows the response does not wait for the copy; it does not drop the client.Measured on an earlier build of this PR:
ACTIVEafterCassandra's copy writes every item through the ordinary put path (a read and a lightweight-transaction write per item, 8 at a time, with the index writes synchronous); that is why it is several times slower than the others. A restore-only plain-insert path is storage-cassandra: restore copy writes every item through the LWT put path #404.
On Cassandra the response waits for the target's schema. With five restores at once it took 0.75 to 1.3 s on an earlier build, and inside the full Cassandra pytest suite it once exceeded 1 s, which is why the test's client timeout is 10 s; the check that the copy runs after the response is that the target is still
CREATINGwhen polled.test_restore_survives_a_client_that_disconnects_before_the_response: a client with a 1 ms read timeout sends the request and closes the connection before any response. The test accepts either outcome that leaves no half-made state: the restore was registered and finishes whole, or the request was cancelled before registration and a new restore into the same name from the same backup is accepted and finishes whole. It prints which. On all four backends the restore was registered and finished. The engine unit testa_restore_request_dropped_while_the_backup_is_read_still_restoresdrops the request future while the backup is being read and checks that the restore still runs.test_target_and_backup_are_held_while_the_copy_runs:DeleteBackup(ExtendDB only) andDeleteTableduring the copy. The status is read immediately before each probe; a probe on aCREATINGtarget must be refused, and each probe must land in the window. On all four backends both probes were refused while the target wasCREATING, and the restore then finished whole.Other new or changed tests: DescribeBackup feature details from an on-demand source and from a
PROVISIONEDsource with SSE (throughput values; theSSEDescriptionassertion is ExtendDB only); ListBackups withLimit=1over three backups (exactly one summary per page and three pages, ExtendDB only; the service's contract allows a cursor on the last page);TableAlreadyExistsException; malformed and foreignBackupArn; empty index overrides, the SSE override, and noSSEDescriptionwithout it; CreateBackup of a vector table; noRestoreSummaryonUpdateTableof anACTIVErestored table; (ExtendDB only) restore of aPROVISIONEDbackup whose GSI has no throughput;ListBackupsfiltered byBackupTypeand a time range; index overrides built fromDescribeBackup(as the console does) and an equalOnDemandThroughputOverride, both restoring; aBackupArnin another partition; and noDeletionProtectionEnabledin the restore response. The DynamoDB-branch message intest_provisioned_restore_override_requiring_gsi_override_leaves_no_targetis the service's reply as measured during review (Invalid Request: Must override ProvisionedThroughput if BillingMode is overridden to PROVISIONED for index by_gpk).tests/test_abac.py::TestBackupResourceTagsreproduces the TagResource case from the review in authenticated mode: a user allowedDescribeBackuponly onaws:ResourceTag/classification = devcannot tag the backup and stays denied (main's #419 refuses the tag operations on a backup ARN; the test pins that against this PR's backups).tests/test_ttl.pygains a TTL-with-GSI test: it requires TTL to be enabled on PostgreSQL, SQLite, and MongoDB (passed on each, with the backend given by the newEXTENDDB_TEST_BACKENDthatdevtools/run-tests --backendexports), and on Cassandra accepts the refusal, which it got at the default delay (ValidationExceptionnamingby_gpkand 10 ms). The Cassandra refusal is pinned by the live storage-level testtest_ttl_on_a_table_with_an_async_gsi_is_refused_as_a_validation_error(refused at 50 ms; enabled once the GSI is synchronous).test_index_overrides_listing_the_backups_indexes_restore_themruns for a PROVISIONED and an on-demand backup and passed on all four backends.tests/test_ttl.py::test_disable_ttlnow waits forENABLINGto end before it disables TTL; it failed once on a loaded PostgreSQL run with "Time to live is being updated".Checks that only ExtendDB can satisfy, and assertions not measured on the service, are skipped against DynamoDB. The file was not run against DynamoDB in this round.
Against older builds: on the round-2 build (8473dd0e), on PostgreSQL, 15 of 19 earlier tests failed, among them the response test with
ReadTimeoutError. On SQLite, the round-3 build before this pass's engine fixes fails theUpdateTabletest (RestoreSummarywithRestoreInProgress: falsein the response) and the GSI-throughput test (ProvisionedThroughput must be specified for index).Further checks with built binaries, all four backends unless noted, on earlier builds of this branch (not repeated on the final binaries):
A SIGKILL mid-restore followed by an immediate restart. The target was removed after 60 s on PostgreSQL, 0.1 s after the restart on SQLite, 60 s on MongoDB, and 62 s on Cassandra. Afterwards
DeleteBackupand a new restore into the name succeeded.A writer updating an early item and then a late item during CreateBackup. 0 pairs out of order on PostgreSQL and SQLite; 16 of 66 on MongoDB and 123 of 800 on Cassandra. Neither MongoDB nor Cassandra takes a point-in-time backup; the docs say so.
Concurrent restores on PostgreSQL: with four restores running in one account, a fifth in the same account waited for a slot (3.1 s) while another account's restore answered in 0.05 s.
Upgrades from origin/main (catalog 0.0.3) with
extenddb migrate:--yesexit 0, preview again exit 0. A table leftCREATINGwas removed at startup and logged by name. Rolling back with the manual's parameterized dump and restore brought the 0.0.3 binary back. A catalog in the pre-release 0.0.4 shape was converged byserveand bymigrate --yes.ListBackupsandDescribeBackupworked;CreateBackup,RestoreTableFromBackup, andDeleteBackupansweredValidationException"Backups are unavailable until the ExtendDB catalog is migrated: it does not record catalog migration V005..." namingextenddb migrate --yes(re-measured on the final binary; an earlier build returned internal errors). Aftermigrate --yes, all of them worked.Backups taken before the upgrade restored afterwards on PostgreSQL, SQLite, and Cassandra.
PostgreSQL
servewith a debug build of this round's PostgreSQL changes: on a catalog whose ledger names another003_*.sqlat 0.0.4, it exits 1 naming003_backup_definitions.sqlandextenddb migrate --yes; aftermigrate --yesit starts. On a pre-release 0.0.4 shape with another session holding a transaction ontable_restores, it exits 1 after about 6 s withcould not lock the table within 5 s; once the lock is free it adds the column, keeps therestore_date_timedefault, and starts, andmigratethen reports only the default step.The
migrateerror text with no admin credentials and with a wrong password (PostgreSQL), and the SQLitepool_sizelog line.The docs mark which statements rest on the code or on storage-level tests rather than on a running server, and which numbers come from an earlier build.
Rust, on the final tree:
cargo test --workspacewithout the PostgreSQL storage crate and the Cassandra live test targets: all passed. extenddb 1, extenddb-app 28, extenddb-auth 219, extenddb-cache 27, extenddb-config 7, extenddb-core 519, extenddb-engine 178, extenddb-server 202, extenddb-storage 125, extenddb-storage-mongodb lib 122 (22 live and model tests ignored), itsinterpreter_selftests18 andpushdown_parity20, extenddb-storage-sqlite 141 (1 ignored); doc-tests 4.cargo test -p extenddb-storage-postgres --tests: lib 53,backup_restore30,catalog_converge5,doc_version_literals3,key_collation3,migrate_data_db_name2,put_create_race9,stream_shard_retention1,twi_conflict8,vector_control_plane42, all passed.cargo test -p extenddb-storage-mongodb --lib -- --ignored --skip protocol_model live_: 18 passed under default test parallelism (three runs in a row before the rebase, one after); the protocol model's tests run with the workspace.cargo test -p extenddb-storage-cassandra --lib: 61 passed;--test direct_integration: 149 passed.test_restore_sweep_worker_sweeps_at_startup_and_stops_on_shutdown, which this PR adds, missed its 8 s deadline on an earlier head on a node that had run tests for about a day (in the full run and alone); on a freshly created node it passed, and it passed in this head's full run. The sweep it exercises has not changed since then.cargo fmt --all -- --check,cargo +1.97.0 clippy --workspace --all-targets -- -D warnings, andcargo +1.88.0 check --workspace --locked --all-targets: clean. In one workspace runextenddb-cache'sstale_hit_serves_cached_and_spawns_refresh, a timing test in a crate this PR does not touch, failed once and then passed five times alone and in the full rerun.The engine, authorization, and the first round of backend fixes were each checked with a mutant (the fix reverted or broken, the new test run; every mutant was caught); of the later backend fixes, the name-before-backup order on SQLite and MongoDB and the cursor and key-info fixes were mutant-checked, the others were not. The MongoDB wedge was reproduced on the build before its fix (20 restores of 20 distinct backups: 6 answered, 14 timed out, every later request timed out) and the build with the fix answered all 20 in 1.1 s with every target
ACTIVEafter 6.9 s (both before the later admission change; the storage-level testlive_twenty_concurrent_restores_of_distinct_backups_do_not_wedge_the_poolruns in the live suite above); the SQLite backup flood was checked at storage level (16 backups, at most 4CREATINGat once, another account's reads unaffected).CI-equivalent legs (
devtools/run-tests):tests/rustintegration passed 512 on PostgreSQL, SQLite, and MongoDB, and the PostgreSQL no-vector leg passed 7, on an earlier build of this branch. The pytest legs (--pytest --comprehensive --parallel) fail exactly the tests that fail on origin/main's binaries on the same host (host Python 3.7 and botocore 1.33): PostgreSQL, SQLite, and MongoDB on the previous build of this branch, and Cassandra on the final one, with 2 workers as CI's 4-core runner gives: 6 failed against main's 6 failed and 1 error (main's extra error is a fixture failure intest_stream_table_reuse.py), plus 4 intests/pythonon both. The PR's Cassandra CI job has failed on two runs, on different tests each time. In the first,test_gsi_configured_delay_rangesaw about 1 s GSI delays; that came from the GSI worker change this revision takes out (#407). In the second, three FGAC tests' teardownDeleteTablereturned 500 because their account's keyspace was missing tables: a race onmainwhen two accounts are created at once (#408), which this PR's extra tests made more likely by changing how the parallel suite is split. Until #408 is fixed, that leg can fail intermittently.Not run: the CLI migration pytest suites (
tests/test_cli_*migrat*.py), because psycopg2 is not installed on the test host.tests/test_put_item_create_race.pyneeds Python 3.8 and was not run on the 3.7 host.Not changed here
RestoreTableFromBackupis still being authenticated or authorized (SigV4 verification and policy evaluation, including the target table's tag reads, run in the request before the engine handler) cancels the request: nothing is created and no error is reported to anyone. From the engine handler on, the request runs in a task of its own. The fidelity suite runs the server in dev mode, where these steps take almost no time, so its 1 ms disconnect test does not exercise this window.backup.rsmaps storageTransientandConnectionerrors to 500InternalServerError, where the generic mapping (for example increate_table.rs) gives 503ServiceUnavailable. Both are retried by the SDKs; worth aligning separately.UpdateTablechecks the 100 projectedNonKeyAttributestotal against a description read before, and outside, the backend's lock, so two concurrent GSICreates can each pass and together exceed it (whether a backend keeps the tableACTIVEwhile the first index isCREATINGwas not checked). A GSI stillDELETINGis also counted.UpdateTableGSICreaterefusesProvisionedThroughputbelow 1 on aPAY_PER_REQUESTtable, while{5, 5}on the same table is still accepted; the "should not be specified when BillingMode is PAY_PER_REQUEST" rule exists only in CreateTable validation. The ">= 1" rule also has three message texts.__typenamespaces forValidationExceptionandAccessDeniedExceptionstill differ from the service's.ListBackupsrequest reads all of the account's (or table's) backups and pages in memory.ProvisionedThroughput{0, 0}for an on-demand backup, which is whatDescribeBackupreports, is refused (leave the member out), as on the service. DuplicateNonKeyAttributesin an index override are accepted (compared as a set); whether the service refuses them was not measured. A GSI entry'sOnDemandThroughputis accepted and dropped:GsiInputhas no field for a per-index on-demand maximum, and adding one touches every backend.GlobalSecondaryIndexOverrideandLocalSecondaryIndexOverrideare accepted only for indexes the backup has, with their key schema and projection unchanged. The reviewer measured the service treating the override list as a redefinition of the restored index set: it accepts and restores an entry naming an index the backup did not have, and one that re-keys an existing index. ExtendDB refuses both withValidationException("is not in the backup", "KeySchema differs"), which is narrower than the service but never produces wrong data. The reviewer is tracking the widening, and the error texts that read differently from the service's while the class is right (among them theTableInUseExceptiontext for a different backup restored into aCREATINGrestore target's name), separately from this PR./healthanswers 200 as soon as the listener is up; it does not probe the storage backend (extenddb healthcheckdocuments it as liveness, not readiness). The condition under which it answered 200 while every request hung (the MongoDB wedge) is removed, and the admission bound keeps half the pool free, but a readiness probe should still issue aDescribeTableorListTableswith a client timeout.UpdateTableon aCREATINGrestore target isResourceInUseExceptionon all four backends; the service answersResourceNotFoundException.ListBackups.TableNameas a table ARN is refused with the pattern clause whileCreateBackupaccepts the ARN form; the service's behaviour forListBackupswas not measured.backup_id_epoch_millisreads the time only from the 14-digit id; the 13-digit ids of earlier builds carry no time for theListBackupscursor placement.DeleteTablecan answer 500; a Cassandra GSI backfill can fail with "Batch too large"; the Cassandra GSI workers underserve(storage-cassandra: asynchronous GSI writes are never applied under serve (default 10 ms delay) #407).CREATING; the backfill worker retries it every 5 s with a warning and has no give-up, and while it isCREATINGDeleteTablerefuses the target andDeleteBackuprefuses its source backup. Not reproduced (no item was found that makes a backfill batch fail persistently); the remedy is to fix the cause and let the worker finish, or to remove the target's catalog document by hand.DeleteBackupthat returned an error can still take effect later. One that failed after claiming the backup (for example on finding a restore from itCREATING) and whose claim could then not be released, neither in the request nor by the claim guard on drop, is finished by the stale-claim sweep once no restore from the backup isCREATING. If that restore is one that never becomesACTIVE(the backfill case above), the claim also blocks new restores of the backup. Not reproduced; it needs two consecutive release failures.extenddb_catalogandextenddb_dataand has no serve lock. The sweeps only act on work whose heartbeat is stale.BackupSizeBytes0.main, Cassandraservenever applies asynchronous GSI writes, and the default delay is 10 ms, so served GSIs miss every write; CI sets the delay to 0), and storage-cassandra: concurrent account creation puts one account's tables in another's keyspace #408 (onmain, concurrent Cassandra account creation can leave an account without its TTL and GSI-queue tables; this is what failed the FGAC tests in this PR's Cassandra CI run). A fix for the GSI worker lifetime was in an earlier revision of this PR and was taken out: reviving the workers exposes defects that belong in storage-cassandra: asynchronous GSI writes are never applied under serve (default 10 ms delay) #407.Unmatched stream id, thenNo nodes available in query plan!on every request until the end), and 90 tests failed and 314 errored. cdrs-tokio 9.0.2 reuses stream ids after 32,767 requests without checking that the old one has been answered, and closes the connection when a second response arrives for an id; it then did not reconnect to the single node. That reading is from the driver's code and the server log, not a reproduction in isolation.devtools/run-cassandra-testsnow setsEXTENDDB_TEST_BACKUP_ITEMSto 10,000 (unless it is already set), which keeps the copy running past the response; the driver behaviour itself is a production risk for a busy server: storage-cassandra: cdrs-tokio drops its only connection under sustained load and never reconnects #403.UpdateTimeToLivereturns an internal error when the backend's TTL change is still blocked by an expiration sweep after its retries (IndexesInUse); SDKs retry it.replication_factor = 1anddatacenter = "datacenter1", whatever--replication-factorwas given.servelogs that background workers did not drain within 5 s.CREATINGuntil the process stops.docs/design/11-high-availability.mdstill suggests transaction-mode pooling for large deployments.table_restores(every DescribeTable) behind it on every server while it waits.REPEATABLE READcatalog snapshot, and so one catalog pool connection and the catalog's xmin, from the first item read until the last full batch is written.CREATINGwith no scheduled transition) and the fallback table ARN built in SQL are written in several SQL statements per backend rather than in one place.test_backup_arn_shape_and_account_are_checkedruns in dev mode, so its texts and its foreign-accountAccessDeniedExceptioncome from the engine. With authorization on, main's pre-authorization check answers "Invalid Backup ARN" for aBackupArnthat does not parse as a backup ARN at all onDescribeBackupandDeleteBackup, before the engine's texts are reached.UpdateContinuousBackupsaccepts enabling point-in-time recovery and reports a 35-day restorable window, although nothing is recorded. This is pre-existing and now documented.Checklist
cargo test --workspace). Not run as one command over every crate: the workspace run without the PostgreSQL storage crate and the Cassandra live targets, and the PostgreSQL, MongoDB, and Cassandra live runs, are listed above and passed. The pytest legs are reported in Testing done.cargo fmt --check)cargo clippy -- -W clippy::pedantic).cargo +1.97.0 clippy --workspace --all-targets -- -D warningsis clean; pedantic was not re-run this round.Storagetrait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Not met; see the next line.ADR / RFC: none accepted. This PR changes the
Storagetrait (BackupEngine) and the on-disk format (catalog 0.0.4, Cassandra V005, SQLitebackup_itemsandtable_restores).docs/rfcs/README.mdrequires an accepted RFC for both, and none exists for these changes. #396 is a design PR, not an RFC: it revisesdocs/design/15-backup-restore.mdfor off-host backup and restore, a series in which it lists this PR as PR 0, to merge first. It describes later changes toBackupEngine(moving create, describe, list, delete, and restore into the engine) and does not specify this PR's change toBackupEngine::restore_table_from_backup. An RFC is required before merge.Breaking changes
SSESpecificationOverride,GlobalSecondaryIndexOverride,LocalSecondaryIndexOverride, andOnDemandThroughputOverrideare applied as the service applies them (an empty index list restores no indexes of that kind; a subset restores those); an unknown or changed index definition, a throughput member contradicting the billing mode,{0, 0}, andBillingModeOverride=PROVISIONEDwith a GSI whose throughput no list entry supplies areValidationException;VectorIndexOverrideis aValidationException. A restore no longer carriesTableClass: the target isSTANDARD.ListBackupsrequests that setBackupTypeSYSTEMorAWS_BACKUP, or a time range, now return only the matching backups.BackupNameandTableNameonCreateBackupandListBackupsare validated (3 to 255 characters,[a-zA-Z0-9_.-]+;TableNametakes the configured table-name limits); aBackupArnorExclusiveStartBackupArnthat is not a well-formed backup ARN isValidationExceptionwhere it wasBackupNotFoundException; a cursor from another account, region, or table, a time bound before the epoch, and a lower bound above the upper bound areValidationException. An ARN of the caller's account in another region now resolves (it wasBackupNotFoundException); a statement in a policy must allow both spellings (below).CreateBackupof a table that does not exist or isCREATINGisTableNotFoundException"Table not found: " (it wasResourceNotFoundException).DescribeBackupanswers an internal error when the definition cannot be read (it answered a description without feature details).DeleteBackupreturns the full description withBackupStatusDELETED.CREATINGrestore target isTableInUseException(it wasTableAlreadyExistsException), and a second restore of a backup while a restore from it isCREATINGisBackupInUseExceptionon every backend (PostgreSQL, SQLite, and Cassandra accepted it).CreateBackupon SQLite is bounded topool_size / 2 - 1at once (4 by default), withLimitExceededExceptionafter 30 s. Restores on MongoDB are bounded to 4 per account andmin(16, pool/2)per server (5 with the driver's default pool), with the same refusal, and a registration that takes longer than 60 s answers an internal error while the restore continues.UpdateTableon a MongoDBCREATINGrestore target isResourceInUseException(it was accepted).CreateTableof aPROVISIONEDtable with a GSIProvisionedThroughputbelow 1 (including 0/0), andUpdateTableGSICreatewith one, are nowValidationException. So are aCreateTablewith more than 100 projectedNonKeyAttributesacross its indexes and anUpdateTableGSICreatethat would take the table over 100.TableAlreadyExistsException, where it wasResourceInUseException.RestoreSummaryonce the table is notCREATING(DescribeTableused to keep it withRestoreInProgress: false, andUpdateTablereturned it too).RestoreDateTimeis now the backup's creation time, not the time the restore ran.RestoreTableFromBackupreturns before the copy. On PostgreSQL and SQLite the table used to beACTIVEwhen the response arrived; now it isCREATINGuntil the copy ends, so callers must wait forACTIVE.pool_sizerestores in total (capped at 16), besides 4 per account; more wait up to 30 s and then getLimitExceededException.extenddb migrateon PostgreSQL migrates the data databaseinitrecorded (or--data-db), not one derived from the catalog database name.DeleteBackupholds isBackupInUseException, and so is aDeleteBackupof a backup another delete holds, where both wereBackupNotFoundException.DeleteBackupthat returns an internal error or is cancelled after it claimed the backup can still complete: the backup readsAVAILABLE,DeleteBackupandRestoreTableFromBackupreturnBackupInUseException, and the stale-claim sweep marks itDELETEDabout 45 to 80 s later (later still if a restore from it isCREATING). Before, such an error released the claim and left the backup available, with no data if the drop had already run. A client that treats an error fromDeleteBackupas "not deleted" is now wrong on MongoDB.ListBackupscursor once deleted or filtered out.BackupCreationDateTimehas millisecond precision.w=0, a numeric or taggedw, orjournal=falseis refused at startup.extenddb migrate --yesis required before serving. PostgreSQL and SQLite servers refuse an unmigrated catalog, and a PostgreSQL server also refuses a catalog whose ledger lacks one of its catalog migration files, even at the current version, and refuses to start a pre-release 0.0.4 catalog if its startup shape step cannot locktable_restoreswithin 5 s. SQLiteservealso refuses a file database in a pre-release 0.0.4 shape untilmigrate --yeshas run. A Cassandra server on an unmigrated catalog serves tables and items, butCreateBackup,RestoreTableFromBackup, andDeleteBackupfail until it is migrated. A stored catalog version that cannot be parsed is now refused rather than overwritten.Storagetrait changed.BackupEngine::restore_table_from_backuptakes a resolvedRestoreRequest,backup_restore_sourceis new, andrestore_table_to_point_in_timeis removed.BackupSummarygainstable_id.extenddb migratepreview without--yesexits 3 when work is pending; it used to exit 1. It exits 0 when everything is up to date.By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.